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

Focus current theme/language in palette #1198

Merged
merged 2 commits into from
Sep 15, 2022

Conversation

alichay
Copy link
Contributor

@alichay alichay commented Sep 14, 2022

Would close #642

The implementation here is a little bit weird, but this is less trivial than it seems

  • If you just set the index in PaletteViewData::run, it gets overridden on the next widget update cycle
  • If you fix that, you still need to have the widget auto-scroll to the selected item. The usual scroll code only happens on update, not creation, and it only fires if the old index != the new index, so this also requires special-casing for when there's a default selection.

If someone has any pointers on how this could be cleaned up or made more idiomatic to the codebase (first contributions are always tricky!) I'd love to hear it. Thanks!

(especially this change to lapce-ui/palette.rs where Druid doesn't implement these shared methods with a trait, so I had to make the function generic over a closure that forwards to the actual method. it's pretty nasty)

@dzhou121 dzhou121 merged commit 05da4b7 into lapce:master Sep 15, 2022
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.

Change Theme menu should focus on current theme
3 participants