Extract Gallery component for semi-cross-platform Gallery block#18265
Merged
Extract Gallery component for semi-cross-platform Gallery block#18265
Conversation
Closed
2a6da85 to
3e5f2ad
Compare
This was referenced Nov 7, 2019
6b4cf2c to
3edb3eb
Compare
cbd2370 to
3effb45
Compare
3edb3eb to
4190b97
Compare
e26655a to
00e5d51
Compare
4190b97 to
8b1e8b3
Compare
00e5d51 to
2eac8cb
Compare
ac55dd8 to
85b45af
Compare
2eac8cb to
7ca3228
Compare
85b45af to
1e403ff
Compare
7ca3228 to
f99f853
Compare
1e403ff to
7a3fa58
Compare
f99f853 to
5a12cc2
Compare
7a3fa58 to
f030539
Compare
396dc3c to
d700b2b
Compare
This reverts commit 2eac8cb.
66670c8 to
6f7d35f
Compare
jorgefilipecosta
approved these changes
Dec 5, 2019
Member
jorgefilipecosta
left a comment
There was a problem hiding this comment.
Things worked well in my tests, and I did not found any regression!
I guess we should try to improve and reduce the needs for platform checks/platform-specific props, but for now, I think this solution is a good step to increase reusability between mobile and web.
Nice work @mkevins 👍
pinarol
reviewed
Dec 5, 2019
pinarol
reviewed
Dec 5, 2019
Contributor
pinarol
left a comment
There was a problem hiding this comment.
@mkevins This is looking good overall, I just entered 2 comments. I want to make a last test tomorrow. Could you update PR branches so that we can have the latest Slider changes as well? After that hopefully we'll be good to go.
2 tasks
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR extracts a
Gallerycomponent for the semi-cross-platform Gallery block. This will allow for web and mobile to share a commonedit.jsfile, which will in turn use the respective web and mobile implementations of theGallerycomponent.PR Hierarchy
This PR is part of the PR hierarchy described here. This PR can be used to test the aggregate changeset and integration of components within the related PRs.
Note:
This PR will need occasional rebasing in order to test the integration with the mobile components in downstream PRs.
Changes
Platformmodule to provide different instructions for adding images on mobileisMobileprop via thewithViewportMatchHOCRangeControlThe icon prop will be hidden for web after the cross-platform PR for
RangeControlis merged (without that merged, icon will be passed through as an attribute to the underlying<input>element). So this PR depends on theRangeControlPR being merged first.BlockIcon:
I've extracted the block icon into a shared-icon file (with web and mobile variants) so that we can apply dark-mode styles on mobile. Also, although the icons use the same underlying svg, they are wrapped differently. Web styles / behaviors should be preserved in this PR.
Separator type:
To make separator indentation consistent for controls in the bottomsheet on mobile, it is currently necessary to add
separatorType="fullWidth"to the controls: https://github.com/WordPress/gutenberg/pull/18265/files#diff-34f83402fe8954f15f8d404d01c9e4c9R302. This can "leak" into the web elements as an attribute as described here: #18155 (comment). I didn't notice any changes in behavior (exposed to the user), but I saw the attribute appearing in the elements. Any guidance on whether this is a blocker for web side is appreciated.This PR should not result in any change in behavior for the web version of Gallery block.
To test
Web:
Checkout this PR and test locally (
npm run env update,npm run dev) and ensure no behavior has changed on web for the Gallery block.Also, all associated automated tests should pass.
Mobile:
Related PRs can be tested via this PR.
Checklist: