Skip to content

Fix #18825: Styles for the toggled state in the ToolbarButton added back #18868

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.

Sign up for GitHub

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

Jump to bottom
Merged
gziolo merged 2 commits into from
Dec 2, 2019
Merged

Fix #18825: Styles for the toggled state in the ToolbarButton added back #18868

gziolo merged 2 commits into from
Dec 2, 2019

Conversation

@gziolo
Copy link
Member

@gziolo gziolo commented Dec 2, 2019

Description

Fixes #18825.

I noticed that this change created regression for toolbar buttons:
toggled-buttons

When they are toggled they no longer change their visual appearance. This is how it worked before 7.0:

Screen Shot 2019-11-29 at 15 38 14

It was introduced in #18631.

An alternative to #18860.

After

Screen Shot 2019-12-02 at 13 10 48

Screen Shot 2019-12-02 at 13 11 15
Screen Shot 2019-12-02 at 13 11 35

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .

@gziolo
Copy link
Member Author

gziolo commented Dec 2, 2019

The only issue I see is:

Screen Shot 2019-12-02 at 13 01 40

However, it's something that exists in the master as well for focused and hovered buttons:

Screen Shot 2019-12-02 at 13 12 58

@gziolo gziolo requested a review from jasmussen December 2, 2019 12:13
@gziolo gziolo mentioned this pull request Dec 2, 2019
Closed
@gziolo gziolo added General Interface Parts of the UI which don't fall neatly under other labels. [Type] Regression Related to a regression in the latest release [Type] Bug An existing feature does not function as intended [Feature] Blocks Overall functionality of blocks labels Dec 2, 2019
@gziolo gziolo self-assigned this Dec 2, 2019
jasmussen
jasmussen approved these changes Dec 2, 2019
View reviewed changes
Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

toggles

Nice. Very nice.

@gziolo
Copy link
Member Author

gziolo commented Dec 2, 2019

I moved the styles to the ToolbarButton component after talking with @youknowriad. In my testing, it works the same, but it's scoped more strictly.

youknowriad
youknowriad approved these changes Dec 2, 2019
View reviewed changes
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM

@gziolo gziolo merged commit b03317e into master Dec 2, 2019
@gziolo gziolo deleted the fix/toolbar-button-toggled branch December 2, 2019 14:17
@youknowriad youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowriad youknowriad youknowriad approved these changes

@jasmussen jasmussen jasmussen approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@chrisvanpatten chrisvanpatten Awaiting requested review from chrisvanpatten

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@talldan talldan Awaiting requested review from talldan

Assignees

@gziolo gziolo

Labels

[Feature] Blocks Overall functionality of blocks General Interface Parts of the UI which don't fall neatly under other labels. [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Projects

None yet

Milestone

Gutenberg 7.1

Development

Successfully merging this pull request may close these issues.

Blocks: Styles for the toggled state in the ToolbarButton removed

4 participants

@gziolo @youknowriad @jasmussen