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

Make divider in contextmenu less aggressive #1228

Merged
merged 1 commit into from
Dec 20, 2022
Merged

Conversation

cisoun
Copy link
Contributor

@cisoun cisoun commented Dec 6, 2022

Hello.

I propose the following change:

  • use divider color instead of caret;
  • add a vertical padding on the divider.

Obviously, the divider is even better if its color is brighter, or darker but it makes sense to me to use the same color as the context menu's border. It will feel less "cheap".

Here's a preview:

menu

Following changes are applied:
- use divider color instead of caret;
- add a vertical padding on the divider.
@TorchedSammy
Copy link
Contributor

i dont mind the color of the divider (i actually like it) but the normal border color with the extra padding looks good too. 🤷‍♂️

@cisoun
Copy link
Contributor Author

cisoun commented Dec 6, 2022

I've just noticed that the autocomplete plugin does not follow this change. It doesn't seem to extend the ContextMenu component, therefore, it would be nice to fix this (if accepted).
I may also have missed other components...

@TorchedSammy
Copy link
Contributor

for autocomplete it's better to leave as is

Copy link
Contributor

@TorchedSammy TorchedSammy left a comment

Choose a reason for hiding this comment

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

looks good tho

@jgmdev jgmdev merged commit 5b5b5fd into lite-xl:master Dec 20, 2022
takase1121 pushed a commit to takase1121/lite-xl that referenced this pull request Jan 16, 2023
Following changes are applied:
- use divider color instead of caret;
- add a vertical padding on the divider.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants