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

Fix missing selected block highlighting in list view #24609

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Aug 18, 2020

Description

Fixes #24520

#22996 accidentally removed the styles for selected blocks in list view. As mentioned in the PR description, this wasn't intended to have any effect on the post editor, only the experimental navigation editor screen.

I've added those styles back and also added some styles for the navigation screen that remove the highlight only on that screen.

How has this been tested?

Post editor

  1. Add some blocks
  2. Select a block
  3. Open List View
  4. The selected block should be highlighted

Nav Screen

  1. Open the experimental navigation screen
  2. Add some blocks
  3. The selected block isn't highlighted

Screenshots

Post editor

Screenshot 2020-08-18 at 12 53 27 pm

Nav Screen

Screenshot 2020-08-18 at 12 54 22 pm

Types of changes

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.

@talldan talldan added [Type] Regression Related to a regression in the latest release [Feature] List View Menu item in the top toolbar to select blocks from a list of links. [Feature] Navigation Screen labels Aug 18, 2020
@talldan talldan self-assigned this Aug 18, 2020
@talldan talldan added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Aug 18, 2020
@github-actions
Copy link

Size Change: +131 B (0%)

Total Size: 1.16 MB

Filename Size Change
build/block-editor/style-rtl.css 10.7 kB +26 B (0%)
build/block-editor/style.css 10.7 kB +25 B (0%)
build/edit-navigation/style-rtl.css 1.12 kB +40 B (3%)
build/edit-navigation/style.css 1.12 kB +40 B (3%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.96 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/index.js 126 kB 0 B
build/block-library/editor-rtl.css 8.36 kB 0 B
build/block-library/editor.css 8.36 kB 0 B
build/block-library/index.js 132 kB 0 B
build/block-library/style-rtl.css 7.47 kB 0 B
build/block-library/style.css 7.48 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.4 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.68 kB 0 B
build/core-data/index.js 11.8 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.56 kB 0 B
build/date/index.js 5.38 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.23 kB 0 B
build/edit-navigation/index.js 10.9 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.62 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.js 11.7 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.33 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@ntsekouras
Copy link
Contributor

Should this highlight the block that the cursor is currently in? If yes, it's not doing it when tested. - Chrome

@talldan
Copy link
Contributor Author

talldan commented Aug 18, 2020

@ntsekouras Thanks for testing. Yep, the icon for the block should display with a black background. Maybe the style is too subtle:
Screenshot 2020-08-18 at 5 26 00 pm

It's also a little easy to confuse with the blue border, which is the focus ring.

@ntsekouras
Copy link
Contributor

It's also a little easy to confuse with the blue border, which is the focus ring.

Yes. I had in mind to look for focus ring :)

Maybe the style is too subtle

IMO it might be yes, especially with black/white only contrast. You might miss that and think a selected icon is the actual icon. But this might be for another follow up issue/discussion.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thanks 👍 !

@talldan
Copy link
Contributor Author

talldan commented Aug 18, 2020

Thanks @ntsekouras, I made a new issue about the design - #24617.

@talldan talldan merged commit 0cbb9e2 into master Aug 18, 2020
@talldan talldan deleted the fix/missing-selected-block-in-list-view branch August 18, 2020 10:25
@github-actions github-actions bot added this to the Gutenberg 8.9 milestone Aug 18, 2020
jorgefilipecosta pushed a commit that referenced this pull request Aug 26, 2020
* Add back missing style for selected block in list view

* Override selected style on navigation screen
@tellthemachines tellthemachines removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] List View Menu item in the top toolbar to select blocks from a list of links. [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

block navigation no longer highlights currently selected block
3 participants