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

Image editing: try transparent zoom control #23646

Closed
wants to merge 1 commit into from

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Jul 2, 2020

Description

Follow up to #23418.
For now it's not possible to put the range control inside the toolbar because it already handles horizontal keyboard navigation. In #23418 an additional toolbar was created, but I find it quite tightly packed for smaller images and it obscures the image.

Before After
Screenshot 2020-07-02 at 14 34 53 Screenshot 2020-07-02 at 14 33 10

Another reason why I like this more is that the image, the scrolling and pinching gesture for zooming are closely connected to the slider control.

zoom-control

How has this been tested?

Screenshots

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.

@ellatrix ellatrix added the [Block] Image Affects the Image Block label Jul 2, 2020
@ellatrix ellatrix requested review from mtias and jasmussen July 2, 2020 11:43
@ellatrix ellatrix requested a review from ajlende as a code owner July 2, 2020 11:43
@github-actions
Copy link

github-actions bot commented Jul 2, 2020

Size Change: -218 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-library/editor-rtl.css 7.53 kB -101 B (1%)
build/block-library/editor.css 7.53 kB -100 B (1%)
build/block-library/index.js 129 kB -17 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 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.42 kB 0 B
build/block-directory/style-rtl.css 941 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 109 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/style-rtl.css 7.79 kB 0 B
build/block-library/style.css 7.79 kB 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 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.2 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 15.9 kB 0 B
build/components/style.css 15.8 kB 0 B
build/compose/index.js 9.65 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/index.js 9.88 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.54 kB 0 B
build/edit-post/style.css 5.54 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3 kB 0 B
build/edit-site/style.css 3 kB 0 B
build/edit-widgets/index.js 9.32 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 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 44.9 kB 0 B
build/editor/style-rtl.css 3.82 kB 0 B
build/editor/style.css 3.82 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.72 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 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 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.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 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.5 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 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

@jasmussen
Copy link
Contributor

I do appreciate how this makes the zoom control work better in constrained spaces, something which would also be solved by moving it to the actual toolbar. But I wonder if there's sufficient contrast here, and I personally really liked the aspect ratio moving out of the toolbar and into context of the zoom bar.

Have we looked into how hard it would be to get the zoom toolbar into the block toolbar itself? I do sense that it's getting very close to sunday here ... but on the other hand, we might end up with an image editing interface that feels frankensteinian :(

@ellatrix
Copy link
Member Author

ellatrix commented Jul 2, 2020

Contrast on white:

Screenshot 2020-07-02 at 17 14 40

Perhaps the "rail" could receive some shadow or an outline?

Have we looked into how hard it would be to get the zoom toolbar into the block toolbar itself?

I don't know honestly. What would the solution be? Use Space/Enter and Esc to move focus into the slider?

@jasmussen
Copy link
Contributor

I don't know honestly. What would the solution be? Use Space/Enter and Esc to move focus into the slider?

Yes, what Diego suggested, that it's behaving like a popover inside the toolbar.

@ellatrix
Copy link
Member Author

ellatrix commented Jul 2, 2020

And we also move the input field inside the toolbar?

@jasmussen
Copy link
Contributor

Yep, and that's why it's important that all other buttons are hidden in this cropping/editing mode.

@ellatrix
Copy link
Member Author

ellatrix commented Jul 3, 2020

Closing in favour of #23677.

@ellatrix ellatrix closed this Jul 3, 2020
@ellatrix ellatrix deleted the try/image-zoom-toolbar branch July 3, 2020 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants