[RNMobile] Remove non-Square layout code from Tiled Gallery mobile feature branch #22320
Conversation
This file contains 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
* Register the block: minimum changes needed for Tiled Gallery to show up and cause no errors * Create a native version of .scss files to avoid bringing in the block's web-specific styles * Add native variant of Tiled Gallery edit: needed because web variant uses DropZone component that is not applicable to native at the moment and generates a warning Co-authored-by: Wendy Chen <wendy.chen@automattic.com>
…1135) * The native mobile version of the Jetpack Tiled Gallery will use inner Image blocks, similar to how the `core/gallery` block now uses inner blocks Co-authored-by: Wendy Chen <wendy.chen@automattic.com>
Add BottomSheet settings Co-authored-by: Wendy Chen <wendy.chen@automattic.com> Co-authored-by: Paul Von Schrottky <paul.von.schrottky@automattic.com>
…1466) * Setting up columns and linkto/rounded corners to sync with the settings selected from bottom sheet. * Adding changelog file. * Delete tiled-gallery-columns Removing changelog since we're merging to a feature branch. * Update settings.native.js Changing max columns to 8 to be in line with mobile gallery block. Co-authored-by: Wendy Chen <wendy.chen@automattic.com>
* [not verified] WIP implementation of save. * [not verified] WIP: Got block attributes to show up in serialization * [not verified] Make columnWidths nested array * [not verified] WIP: Now converts serialized HTML images to inner blocks on load if they weren't already in store. * [not verified] WIP load from save. Hacked in stuff to make it work. Need to find root cause. * Adding Platform checks to ensure that mobile changes don't affect web. Fixing some lint warnings. Co-authored-by: Wendy Chen <wendy.chen@automattic.com> Co-authored-by: Paul Von Schrottky <paul.von.schrottky@automattic.com>
* Register the block: minimum changes needed for Tiled Gallery to show up and cause no errors * Create a native version of .scss files to avoid bringing in the block's web-specific styles * Add native variant of Tiled Gallery edit: needed because web variant uses DropZone component that is not applicable to native at the moment and generates a warning Co-authored-by: Wendy Chen <wendy.chen@automattic.com>
…1135) * The native mobile version of the Jetpack Tiled Gallery will use inner Image blocks, similar to how the `core/gallery` block now uses inner blocks Co-authored-by: Wendy Chen <wendy.chen@automattic.com>
Add BottomSheet settings Co-authored-by: Wendy Chen <wendy.chen@automattic.com> Co-authored-by: Paul Von Schrottky <paul.von.schrottky@automattic.com>
…1466) * Setting up columns and linkto/rounded corners to sync with the settings selected from bottom sheet. * Adding changelog file. * Delete tiled-gallery-columns Removing changelog since we're merging to a feature branch. * Update settings.native.js Changing max columns to 8 to be in line with mobile gallery block. Co-authored-by: Wendy Chen <wendy.chen@automattic.com>
* [not verified] WIP implementation of save. * [not verified] WIP: Got block attributes to show up in serialization * [not verified] Make columnWidths nested array * [not verified] WIP: Now converts serialized HTML images to inner blocks on load if they weren't already in store. * [not verified] WIP load from save. Hacked in stuff to make it work. Need to find root cause. * Adding Platform checks to ensure that mobile changes don't affect web. Fixing some lint warnings. Co-authored-by: Wendy Chen <wendy.chen@automattic.com> Co-authored-by: Paul Von Schrottky <paul.von.schrottky@automattic.com>
…tic/jetpack into rnmobile/add/tiled-gallery-block
Co-authored-by: Wendy Chen <wendy.chen@automattic.com>
* Pass 'squared' class back to image for styling * Crop images in Tiled Gallery by default By adding an "imageCrop" and setting it to true, images in the Tiled Gallery will now appear cropped by default. This is important so that images don't appear unusually distorted when their shape is changed to be square. * Hide image block's caption component As the image block's caption component is not a part of the Tiled Gallery block's design, it's hidden with this commit. This is done by adding an "hideImageCaption" context and setting it to true. This is passed back to the image block, which is set to hide the component when this context is detected. * Remove uncessary code "imageCrop" and "hideImageCaption" don't need to be set as props. * Set aspect ratio of images so that they are squares With this commit, the image's aspect ratio is being set in Jetpack's CSS files. It was previously set in Gutenberg's files, but this wasn't inline with our guidelines for Jetpack code appearing in the open source project. * Remove caption, as it's unused * Fix reference to CSS class The previous version of this code wasn't working as expected, as it included quotation marks. With this commit, the issue is fixed and the reference to the CSS class now works as expected. * Remove 'hideImageCaption' context from PR's scope In order to limit the PR's scope to the square aspect ratio, this commit removes the 'hideImageCaption'. New contexts for hiding the image block's caption and settings will be moved to #21278. * Create native-specific index file to avoid impact to web * Remove unused contexts from web-only index file As a native-specific index file was introdued in 128667a, we no longer need to include contexts in the web-only index file, as they're only used with the apps. * Revert unecessary code removal The removal of the image's caption goes beyond the scope of this specific PR, so its removal is reverted with this PR. * Combine native-specific file into shared file As discussed here, there isn't currently a compelling need to keep the changes to the index file in a native-specific file: #21166 (comment) This change therefore reverts the use of a native-specific file, combining its changes into the shared index file. * Remove layout constant, for clarity As the layout constant doesn't currently have any values other than "squared", it's removed with this commit.
With this PR, the placeholder text is changed from "ADD IMAGE" to "ADD MEDIA". This is to bring consistency with the text that's seen in other, similar blocks, such as the Gallery and Story blocks.
Formerly `__experimentalUseInnerBlocksProps`, the API has now become `useInnerBlocksProps` so we update usage.
The Tiled Gallery's icon color is not the same as other blocks. By removing 'fill="currentColor"' in this PR, the block icon will go back to the general default color that's used for all block icons, thus restoring consistency.
…bile serialized HTML can match. (#21643) Co-authored-by: Wendy Chen <wendy.chen@automattic.com>
* Add basic layout picker to Tiled Gallery in settings sub-sheet
This PR leverages the `__experimentalHideChildBlockControls` block supports introduced in WordPress/gutenberg#36371 to hide the settings tab for the inner image blocks.
…21884) For the current phase of the Tiled Gallery block, we won't be shipping the block with "link to" settings. As such, this PR removes all references to those settings.
This PR includes the following functionality related to columns in the Tiled Gallery block: * Allows users to set the columns to either the number of images in the gallery (up to the web's max value of 20). * The number of columns displayed within the editor itself is set to a max of four for landscape and two for portrait. * The columns resize depending on whether they're in portrait or landscape mode.
Removes a hard-coded dimension in the Tiled Gallery save function that was put there while iterating on the development of the block.
github-actions
bot
added
[Block] Tiled Gallery
[Plugin] Jetpack
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Jetpack plugin:
|
github-actions
bot
added
the
[Status] Needs Author Reply
guarani
changed the title
SiobhyB
reviewed
Jan 12, 2022
| name: LAYOUT_DEFAULT, | ||
| }, | ||
| { | ||
| name: LAYOUT_CIRCLE, | ||
| }, | ||
| { | ||
| isDefault: Platform.OS === 'native', |
There was a problem hiding this comment.
@guarani, I came across an issue when testing this across web/native. If you create a gallery on native, it's correctly set to the square layout, but this is undone if you open the editor in a web browser:
- Create a gallery in the app and confirm it's correctly set to the square layout.
- Save the post and then reopen its editor in a web browser.
- See how the gallery automatically converts to the tiled mosaic layout.
There was a problem hiding this comment.
I think this is related to an issue I'm seeing where the className is not set in the block's serialized representation on mobile. The image below shows web
Web (includes className) |
Mobile (missing className) |
|---|---|
 |
 |
SiobhyB
requested changes
Jan 12, 2022
There was a problem hiding this comment.
Thanks for creating this, Paul! I know this is still a WIP but wanted to give a heads up that, in addition to this issue, the images in the gallery no longer appear as squares when previewed with this PR checked out:
Let me know if you're unable to replicate or if I can help with anything else here.
|
I'm closing this PR since we decided to leave other layouts in #21849. |
github-actions
bot
removed
the
[Status] Needs Author Reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes proposed in this Pull Request:
Jetpack product discussion
p9ugOq-1Tb-p2
Does this pull request change what data or activity we track or use?
No, it does not.
Testing instructions:
Verify that the Square layout works on mobile and that other layouts are not available.
Setup to test from the main apps
add/tiled-gallery-blockbranch:npm run start:reset