Skip to content

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

Merged
gziolo merged 2 commits into
masterfrom
fix/toolbar-button-toggled
Dec 2, 2019
Merged

Fix #18825: Styles for the toggled state in the ToolbarButton added back#18868
gziolo merged 2 commits into
masterfrom
fix/toolbar-button-toggled

Conversation

@gziolo

@gziolo gziolo commented Dec 2, 2019

Copy link
Copy Markdown
Member

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

gziolo commented Dec 2, 2019

Copy link
Copy Markdown
Member Author

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

@jasmussen jasmussen left a comment

Copy link
Copy Markdown
Contributor

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

gziolo commented Dec 2, 2019

Copy link
Copy Markdown
Member Author

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

@youknowriad youknowriad left a comment

Copy link
Copy Markdown
Contributor

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

3 participants

@gziolo @youknowriad @jasmussen