-
Notifications
You must be signed in to change notification settings - Fork 109
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
Show which item is selected in SelectionList
#48
Show which item is selected in SelectionList
#48
Conversation
@@ -50,12 +50,12 @@ default = [ | |||
] | |||
|
|||
[dependencies] | |||
iced_style = { git = "https://github.com/hecrj/iced.git", rev = "adce9e0" } | |||
iced_style = { git = "https://github.com/iced-rs/iced.git", rev = "adce9e0" } |
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 change breaks every example (at least for me). This change would need to be taken on every example as well
Would you like to open up another PR with this changes and remove this change from this PR for now?
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.
yeah id request that he removes this change from his PR so we can go fix these in a separate PR.
selected_text_color: Color::WHITE, | ||
selected_background: Background::Color([0.4, 0.4, 1.0].into()), | ||
selected_background: Background::Color([0.0, 1.0, 0.0].into()), |
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.
The background color's lightness is nearly indistinguishable from the color of the text. The text is really hard to read. Could you use another color like a dark green for the background or a more darker color like black for the text?
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.
Thank you very much for your PR! 🙂
However, there are two small change requests that need to be solved before merging.
This PR implements an easy way to tell which item in a
SelectionList
is currently selected by tracking the index of the currently selected item and styling it accordingly.Previously,
Style.selected_text_color
andStyle.selected_background
defined the styling of the item that the mouse was hovering over. These have been renamed toStyle.hovered_text_color
andStyle.hovered_background
, respectively, andStyle.selected_text_color
andStyle.selected_background
now accurately refer to the styling applied to the currently selected item.I have verified that the
selection_list
example still works - no code changes were required.