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

Highlight Toggled State of Toolbar Items #8968

Merged
merged 2 commits into from
Jan 28, 2021

Conversation

seantan22
Copy link
Contributor

@seantan22 seantan22 commented Jan 19, 2021

What it does

Fixes #8312

  • Renders toolbar items with a highlighted background if in a toggled state.
  • Adds a sample toolbar item (add icon) to the sample view toolbar that can be toggled for testing.

How to test

  1. Go to View -> Sample Unclosable View
  2. Click on the toolbar item (add/plus icon) to toggle and
    observe that a highlighted background appears behind the toolbar icon.

image

image

Review checklist

Reminder for reviewers

Signed-off-by: seantan22 [email protected]

@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves toolbar issues related to the toolbar labels Jan 19, 2021
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Works well for me, and the code looks good for the most part, with a couple of minor issues.

What It Does

Adds a sample toolbar item (add/plus icon) to the sample view toolbar that can be toggled.

Signed-off-by: seantan22 <[email protected]>
@seantan22 seantan22 force-pushed the toggle-toolbar-item branch from 0388507 to 5088583 Compare January 19, 2021 17:52
Fixes [eclipse-theia#8312](eclipse-theia#8312)

What It Does
- Renders toolbar items with a highlighted background if in a toggled
  state.

How To Test
1. Go to View -> Sample Unclosable View
2. Click on the toolbar item (add/plus icon) to toggle and observe that a highlighted background appears behind the toolbar icon.

Signed-off-by: seantan22 <[email protected]>
@seantan22 seantan22 force-pushed the toggle-toolbar-item branch from 5088583 to f3482c5 Compare January 19, 2021 20:01
@colin-grant-work colin-grant-work self-requested a review January 20, 2021 17:11
@colin-grant-work
Copy link
Contributor

The code looks good to me. @vince-fugnitto or @marechal-p, would you be willing to give it a once over to make sure it isn't doing something in the example views that shouldn't be done?

@colin-grant-work colin-grant-work self-requested a review January 20, 2021 17:20
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good to me 👍
I can merge the updates tomorrow if there are no objections.

@vince-fugnitto vince-fugnitto merged commit 0445596 into eclipse-theia:master Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement issues that are enhancements to current functionality - nice to haves toolbar issues related to the toolbar
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visually highlight the toggled state of toolbar items
3 participants