Move some editor styling to floem style props #398
Conversation
There was a problem hiding this comment.
Overall I like the idea/impl.
I was initially envisioning the floem style being inside SimpleStyling, but this is fine and works better in various cases.
Somewhat uncertain about changing phantom_text to take &EditorStyle instead. I was initially envisoning it as "user can downcast &dyn Styling if they have a custom impl to get arbitrary styling information".
That's what I'd have done in Lapce if we didn't already have config on our document for phantom text coloration (because lapce's phantom text has things like diagnostics and background color).
But I think that's fine enough.
I think it would be good to have some way of doing a decent default dark/light theme, since that seems no longer used.
(if we're going with "floem should just have very basic styles", I'm admittedly a bit skeptical of that)
src/views/editor/view.rs
Outdated
| EditorView::paint_text(cx, &ed, viewport, &screen_lines, &self.editor_view_style); | ||
|
|
||
| // TODO: what is going on here? Why is this necessary | ||
| EditorView::paint_scroll_bar(cx, viewport, &self.editor_view_style); |
There was a problem hiding this comment.
Lapce paints its own scrollbar.
(initially the editor scrollbar was broken in floem, then someone did something that made it weirdly thick but working properly, so I didn't bother looking at it more)
There was a problem hiding this comment.
(Possibly we could/should just remove the custom scrollbar painting code from floem-editor as Lapce has to write its own function on top for extra git map stuff anyway. Unsure)
There was a problem hiding this comment.
(Possibly we could/should just remove the custom scrollbar painting code from floem-editor as Lapce has to write its own function on top for extra git map stuff anyway. Unsure)
I'll look at removing it would be more consistent with the rest of floem if it just used the one from the scroll view.
I was planning on using the colors from the previous SimpleStyling::light and dark and turning them into floem::styles that can be easily applied |
|
I figured out how to revert the whitespace changes so this should clearer to review now. |
|
I've fixed several issues that I had and now need to actually go back and add docs and update the examples |
| GutterStyle { | ||
| accent_color: TextColor, | ||
| dim_color: DimColor, | ||
| left_padding: LeftOfCenterPadding, |
There was a problem hiding this comment.
added the ability to customize the gutter padding
| @@ -152,14 +184,44 @@ impl Widget for EditorGutterView { | |||
| let mut text_layout = TextLayout::new(); | |||
| if line == current_line { | |||
| text_layout.set_text(&text, current_line_attrs_list.clone()); | |||
| if let Some(current_line_color) = self.gutter_style.current_line_color() { | |||
There was a problem hiding this comment.
Added the ability to have the gutter show the highlighted line. This appears continuous with the editor highlighted line.
|
That was a lot more work than I was expecting when I started. But now everything but line/syntax specific properties in the editor can be styled using floem styles and the editor_style method makes it super easy to use and obvious what is stylable. |
|
This fixes #393 no? Or are you expecting to move even more styles over? |
|
Oh yeah I just forgot to reference it as fixed. This fixes it |
| @@ -359,50 +356,64 @@ pub trait Styling { | |||
| } | |||
| } | |||
|
|
|||
| pub fn default_light_color(color: EditorColor) -> Color { | |||
| pub fn default_light_theme(mut style: EditorCustomStyle) -> EditorCustomStyle { | |||
There was a problem hiding this comment.
Name for light variant was changed but name of dark variant wasn't. Is there some reason for the difference or why was it changed?
This works and I think works really well.
I'm marking this as a draft because I still need to add docs and update the examples.
@MinusGix review on this would be good especially for things that might need to be changed to work with Lapce.
Mostly that is around SimpleStyling which now is only for properties that require line information