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.

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

Tiled gallery: add next-prev image order buttons #14702

Merged
merged 3 commits into from
Feb 17, 2020
Merged

Tiled gallery: add next-prev image order buttons #14702

merged 3 commits into from
Feb 17, 2020

Conversation

simison
Copy link
Member

@simison simison commented Feb 16, 2020 •
edited
Loading

Resolves #14546

This was a pretty quick one by mostly copying from the core. Not entirely sure how to give credit back.

Changes proposed in this Pull Request:

  • Add next/prev buttons
  • Adjust close-button style to match core gallery:

Before
Screenshot 2020-02-16 at 23 00 08

After
image

Mostly just copied code from core gallery to align. This could be re-usable somehow from the core but I think using inner blocks for the UI would be a better approach than sharing components for this (see example "buttons" block).

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • Improve! 💪

Testing instructions:

  • Add tiled gallery and add some images to it
  • Ensure you can still re-arrange it via regular media modal:
    Screenshot 2020-02-15 at 18 42 40
  • Ensure you can re-arrange images via arrow buttons — test all layouts:
    Screenshot 2020-02-15 at 18 41 21
  • Ensure first and last image have disabled next/prev buttons:
    Screenshot 2020-02-15 at 18 41 52
  • Ensure very small images work
    Screenshot 2020-02-15 at 18 41 34
  • Ensure arrows switch to up/down arrows when using only one column layout
    Screenshot 2020-02-16 at 22 58 38

Proposed changelog entry for your changes:

  • Easily re-arrange images in Tiled gallery

@simison simison added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Block] Tiled Gallery labels Feb 16, 2020
@simison simison requested a review from a team February 16, 2020 21:03
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello simison! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D38971-code before merging this PR. Thank you!

@jetpackbot
Copy link

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: February 17, 2020.
Scheduled code freeze: February 10, 2020

Generated by 🚫 dangerJS against af803bd

simison
simison commented Feb 16, 2020
View reviewed changes
extensions/blocks/tiled-gallery/gallery-image/edit.js
</div>
) }
<div className="tiled-gallery__item__move-menu">
<IconButton
Copy link
Member Author

@simison simison Feb 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Deprecated <IconButton> can be changed to <Button icon> once it's available in WP.

#14668

@simison
Copy link
Member Author

simison commented Feb 16, 2020 •
edited
Loading

image

I disagree with most of these (it's silly, flagging SVG blobs as similar and such...) and I'm not gonna start doing completely unrelated refactoring in a very focused feature PR and start risking adding new bugs to what is pretty well-tested code at this point. Thanks for understanding! :-)

@jeherve
Copy link
Member

jeherve commented Feb 17, 2020

I disagree with most of these

That's alright. This is still a work in progress, we need to continue to tweak those rules until all recommendations given are more relevant. Until then, those are mostly suggestions, it's okay to ignore them. :)

@jeherve jeherve added this to the 8.3 milestone Feb 17, 2020
jeherve
jeherve approved these changes Feb 17, 2020
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This worked well in my tests! 🚢

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 17, 2020
@simison simison merged commit 0b5fab9 into master Feb 17, 2020
@simison simison deleted the update/tiled-gallery-order branch February 17, 2020 13:10
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 17, 2020
@simison
Copy link
Member Author

simison commented Feb 17, 2020 •
edited
Loading

Synced in r202962-wpcom

This was referenced Feb 17, 2020
Merged
Merged
jeherve added a commit that referenced this pull request Feb 25, 2020
@jeherve
Changelog: add #14702
1e32fbb
jeherve added a commit that referenced this pull request Feb 25, 2020
@jeherve
Changelog: 8.3 running changelog (#14642)
8797bea 
* 8.3 release: changelog

* Changelog: add #14516

* Changelog: add #14574

* Bring in changes from 8.2.1 and 8.2.2

* Update stable version

* Bring in 8.2.3 changes

* Changelog: add #14714

* Changelog: add #14639

* Changelog: add #14678

* Changelog: add #14673

* Changelog: add #14687

* Changelog: add #14704

* Changelog: add #14702

* Changelog: add #14541

* Changelog: add #14657

* Changelog: add #14622

* Changelog: add #14582

* Changelog: add #14638

* Changelog: add #14633

* Changelog: add #14571

* Changelog: add #14592

* Changelog: add #14539

* Changelog: add #14514

* Changelog: add #14643

* Changelog: add #14494

* Changelog: add #13739

* Changelog: add #14707

* Changelog: add #14736

* Changelog: add #14706

* Changelog: add #14730

* Changelog: add #14685

* Changelog: add #14727

* Changelog: add #14711

* Changelog: add #14742

* Changelog: add #14746

* Changelog: add #14725

* Changelog: add #13999

* Changelog: add #14740

* Changelog: add #14759

* Changelog: add #14703

* Changelog: add #14753

* Changelog: add #14754

* Changelog: add #14645

* Cahngelog: add #14599
@jeherve jeherve mentioned this pull request Mar 5, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

@simison simison

Labels
[Block] Tiled Gallery Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
8.3
Development

Successfully merging this pull request may close these issues.

Blocks: It's not possible to rearrange images in a tiled gallery block
4 participants
@simison @matticbot @jetpackbot @jeherve