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

Tiled gallery: rounded corners #14704

Merged
merged 4 commits into from
Feb 17, 2020
Merged

Conversation

simison
Copy link
Member

@simison simison commented Feb 16, 2020

Adds rounded corners feature to tiled gallery block.

Changes proposed in this Pull Request:

  • Rounded corners. 🎉

Screenshot 2020-02-16 at 23 16 16

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

  • 💪 enhance

Testing instructions:

  • Add tiled gallery block and some images to it

  • Confirm that you can make images nicely round with the sidebar setting.

    Screenshot 2020-02-16 at 23 16 57
  • Oh, except with Circle layout which obviously is already pretty darn round.
    Screenshot 2020-02-16 at 23 18 46

  • Store galleries without this change and try to open them with this change; confirm nothing breaks.

  • Test the roundness at the frontend of the site

Proposed changelog entry for your changes:

  • Allow adding rounded corners to Tiled gallery images

@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:35
@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 D38974-code before merging this PR. Thank you!

@@ -33,6 +34,12 @@ $tiled-gallery-max-column-count: 20;
display: flex;
}
}

@for $i from 1 through $tiled-gallery-max-rounded-corners {
Copy link
Member Author

@simison simison Feb 16, 2020

Choose a reason for hiding this comment

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

This will generate 20 lines of CSS like this. Figured it's lightweight enough addition when limited to 20 or so.

Alternatively (maybe in another PR) we can have this in a separate CSS file which we enqueue only when they have rounded corners

Another alternative is to generate this in PHP for the exact number as inline CSS, allowing the setting to become unlimited instead of capped to 20.

@jetpackbot
Copy link

jetpackbot commented Feb 16, 2020

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: March 3, 2020.
Scheduled code freeze: February 25, 2020

Generated by 🚫 dangerJS against b5776a6

@@ -128,6 +128,10 @@ const blockAttributes = {
default: 'none',
type: 'string',
},
roundedCorners: {
Copy link
Member Author

Choose a reason for hiding this comment

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

borderRadius could be another name and match CSS rule, but figured this is more self-explanatory.

@@ -3,6 +3,7 @@
*/
import { __, sprintf } from '@wordpress/i18n';
import { Component } from '@wordpress/element';
import classnames from 'classnames';
Copy link
Member Author

@simison simison Feb 16, 2020

Choose a reason for hiding this comment

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

Confirmed that this doesn't end up to built bundle meant for frontend _inc/blocks/tiled-gallery/view.js and make it needlessly bigger.

@@ -12,6 +13,7 @@ import GalleryImageSave from '../gallery-image/save';
import Mosaic from './mosaic';
import Square from './square';
import { isSquareishLayout, photonizedImgProps } from '../utils';
import { LAYOUT_CIRCLE, MAX_ROUNDED_CORNERS } from '../constants';
Copy link
Member Author

Choose a reason for hiding this comment

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

Confirmed that this doesn't pull in wp.i18n dependency on frontend bundle.

@matticbot
Copy link
Contributor

simison, Your synced wpcom patch D38974-code has been updated.

@jeherve jeherve added this to the 8.3 milestone Feb 17, 2020
jeherve
jeherve previously approved these changes Feb 17, 2020
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.

Nice addition! 👍

@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
@matticbot
Copy link
Contributor

simison, Your synced wpcom patch D38974-code has been updated.

@simison
Copy link
Member Author

simison commented Feb 17, 2020

Had to rebase after #14702; screenshots now little stale (but not much) and I'll need another ✅ from you @jeherve if you don't mind. :-) Thanks!

@simison simison merged commit a3f477b into master Feb 17, 2020
@simison simison deleted the update/tiled-gallery-rounded-corners branch February 17, 2020 13:30
@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

Synced in r202963-wpcom

@simison simison restored the update/tiled-gallery-rounded-corners branch February 17, 2020 13:39
@simison simison deleted the update/tiled-gallery-rounded-corners branch February 17, 2020 13:42
jeherve added a commit that referenced this pull request Feb 25, 2020
jeherve added a commit that referenced this pull request Feb 25, 2020
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants