Update gallery images to include post id #1469
Conversation
|
An issue that still exists - inserting a Gallery from WordPress Core does not include the postId - so if a gallery is inserted with current Editor and then edited in Gutenberg, the images won't get loaded properly. Likewise, how much of Gutenberg gallery code should match existing gallery code? We can duplicate it which should get Gutenberg as backward compatible, so a gallery created in Gutenberg shows as gallery in current Editor. |
| @@ -2,7 +2,7 @@ | |||
| export default function GalleryImage( props ) { | |||
| return ( | |||
| <figure className="blocks-gallery-image"> | |||
| <img src={ props.img.url } alt={ props.img.alt } /> | |||
| <img src={ props.img.url } alt={ props.img.alt } data-postId={ props.img.id } /> | |||
There was a problem hiding this comment.
This sounds like something we'd want as attributes in the comment.
There was a problem hiding this comment.
Going to keep the attribute inline, since it adds complexity to include data in comment since would require duplicating the data set and then parsing out isn't
| @@ -1,10 +1,10 @@ | |||
| <!-- wp:core/gallery --> | |||
| <div class="blocks-gallery wp-block-gallery"> | |||
| <figure class="blocks-gallery-image"> | |||
| <img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" /> | |||
| <img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" id="1" /> | |||
There was a problem hiding this comment.
This would quickly run into conflicts, no?
There was a problem hiding this comment.
I missed updating the fixture with data-postId, but looks like it should switch toan attribute in comment instead.
|
Side note: I updated the |
mkaz
force-pushed
the
fix/1445-gallery-id
branch
from
65514e4
to
cf7bbb4
Compare
|
@youknowriad I'll look at using that value prop for #1447 - the Gallery currently uses the |
|
It turns out more than just This PR switches the way attributes are stored to use comments, but slimmed down the set to store only those needed, not all that get returned. To test - after creating a gallery save, and then close and reopen the post fresh. Testing it without refreshing or opening will include all the attributes the Media Modal sends back, not just what is stored. This was the error before why I thought it was working. |
|
Why are all those needed? We don't want to use the media library sidebar panel for things like alt/url/size, etc. In fact we'd want to just disable that sidebar and use our inspector only. |
mkaz
force-pushed
the
fix/1445-gallery-id
branch
from
8c2ffc1
to
e5936b2
Compare
The unit tests fail if the element has a attribute called type, since that
triggers it thinking it is a React Tree. Adds an additional check for
element.props which is assumed exists causing the error below.
```
1) full post content fixture core-gallery:
TypeError: Cannot read property 'children' of undefined
at normalizeReactTree (build/test.js:8785:20)
at build/test.js:8770:11
at Array.map (native)
at normalizeReactTree (build/test.js:8769:18)
at build/test.js:8804:26
at Array.map (native)
at normalizeParsedBlocks (build/test.js:8795:16)
at Context.<anonymous> (build/test.js:8816:33)
```
mkaz
force-pushed
the
fix/1445-gallery-id
branch
from
7b9e527
to
538bbb9
Compare
|
The duplication of information between comment attributes and HTML markup introduced here looks quite bad, and is inviting a lot of problems related to one source falling out of sync with the other. We need to pick one method or the other of storing data. |
|
@nylen open to suggestions on how not to duplicate the data? |
|
Sorry, I have been meaning to look more closely at this but haven't had a chance yet. See also #1986. |
…rg-mobile into issue/AztecAndroid-IndexOutOfBoundsException-setSpan * 'develop' of https://github.com/wordpress-mobile/gutenberg-mobile: (50 commits) Update issue templates Update gutenberg ref Update gutenberg ref Update issue templates Update GB ref to point to official 1.16.0 tag Update RELEASE-NOTES.txt Update JS bundles Update GB ref Update app bundles Update GB reference. [Aztec iOS]: `shouldInteractWith` will return always true to avoid crashes [Aztec iOS] Cleanup white spaces. Update bundles. Update version to 1.6.0 Updated bundles Force translation update when generating new bundles Update gutenberg ref 720-Add support for giphy and pexel images (#1469) Update bundles Update gutenberg ref ... # Conflicts: # gutenberg
Fixes #1445
We need to send along to Media Modal the images with the post id, so when editing a gallery the dialog opens with already selected images.
Storing postId in image tag using data-postId to not confuse it with CSS id