-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Move Activity Bar Settings to Activity #17457
Move Activity Bar Settings to Activity #17457
Conversation
xref #17284 |
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.
Nice, looks good!
@@ -43,7 +45,7 @@ const emit = defineEmits(["goToAll"]); | |||
height: 100%; | |||
display: flex; | |||
flex-flow: column; | |||
padding: 0.5rem 0.25rem; | |||
padding: 0.5rem 1rem; |
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.
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 should be the same spacing as the tool box. I'm open to change it, but if we do we should do so for the tool box as well
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 would prefer to use the activity panel for the tool box as well to have a consistent UI.
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 sounds like a different and controversial option ... can we merge without that ?
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'd prefer keeping it aligned with existing styles for now, and making any style changes in a separate PR.
That way we can discuss there in more detail
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.
Moves the settings from a context menu to the settings activity.
Additional changes:
How to test the changes?
(Select all options that apply)
License