-
Notifications
You must be signed in to change notification settings - Fork 37
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
WIP: Set kern group #244
base: master
Are you sure you want to change the base?
WIP: Set kern group #244
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I think this should work, although it's definitely a bit hairy 😅
@@ -356,6 +358,33 @@ impl Workspace { | |||
} | |||
} | |||
|
|||
pub fn set_glyph_group(&mut self, prefix: &str, glyph_name: GlyphName, group_name: String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definitely reenforces my suspicion that we should find some better way to store these. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, though it's hard to think of a good clean place to introduce yet another intermediate datastructure
@@ -63,13 +63,13 @@ fn selected_glyph_widget() -> impl Widget<GlyphDetail> { | |||
.with_child( | |||
Flex::row() | |||
.with_child( | |||
Label::new("kern group") | |||
Label::dynamic(|d: &GlyphDetail, _| (*d.kern2_group).to_string()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so getting the editing working should be a two step process:
-
use
EditableLabel
:EditableLabel::new( |s: &String, _: &_| s.to_string(), |s| Some(s.to_string()) ) .lens(GlyphDetail::kern1_group)
-
Add some controller that sends a message to rename the glyph when the data changes.
That should get this working, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I thought, though it involves writing two more dummy lenses in GlyphDetail I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that sounds right. I don't love it, and also, it works,
No description provided.