Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A foundation for text editing and selection in Masonry #241

Merged
merged 17 commits into from
May 3, 2024

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented May 2, 2024

This brings in a lot of the old work from Druid and Masonry (prior to linebender/masonry#56) on text, as well as some types from Glazier.

Needed work:

  • Text display using abstract types
  • Text selection with mouse
  • Text input with keyboard
  • IME integration (of the kind winit understands)

Follow up work:

  • Keyboard control of the selection area (hard-coded hotkeys)
  • Proper placement of cursor
  • Input methods
  • Proper hotkey handling
  • Copy and/or paste

crates/masonry/src/text2/layout.rs Outdated Show resolved Hide resolved
crates/masonry/src/text2/layout.rs Outdated Show resolved Hide resolved
crates/masonry/src/text2/layout.rs Outdated Show resolved Hide resolved
//TODO: add inking_rect
}

impl<T> TextLayout<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a general comment that some things are prefixing with set_ and others with set_text_ and sometimes set_color is used in here and in other places set_text_color and it might be nice to make this more consistent. (Probably not immediately before RustNL...)

Copy link
Member Author

@DJMcNab DJMcNab May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, all of this API design could do with a tiny bit of cleanup; there's quite a lot of redundancy in these methods

@DJMcNab DJMcNab marked this pull request as ready for review May 3, 2024 14:50
Copy link
Member

@xStrom xStrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving in the interest of v0.1 for RustNL, with the understanding that the code introduced here is buggy and is missing functionality that will be addressed in future PRs.

@DJMcNab DJMcNab added this pull request to the merge queue May 3, 2024
Merged via the queue into linebender:main with commit 52bbfa0 May 3, 2024
7 checks passed
@DJMcNab DJMcNab deleted the masonry-text branch May 3, 2024 16:00
github-merge-queue bot pushed a commit that referenced this pull request May 5, 2024
This is fallout from #241. As discussed on Zulip, using different
underlying text types here is a very advanced feature, which will not be
needed by any real consumers. It also creates some footguns

Probably conflicts with #257, so that should land first
github-merge-queue bot pushed a commit that referenced this pull request Jul 18, 2024
This was imported in #241

See also [#masonry > text vs
text2](https://xi.zulipchat.com/#narrow/stream/317477-masonry/topic/text.20vs.20text2)

We do want to sort out text properly, see #337, so I'm keeping the
reference code around.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants