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.
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
Gallery: Convert Gallery block to use Image blocks instead of having its own nested image format #25940
Gallery: Convert Gallery block to use Image blocks instead of having its own nested image format #25940
Changes from 146 commits
c01bbfb
038b22d
331c302
ee7da0a
c9fe911
a00a544
0c8742f
a77854c
1965514
e5e1420
1d6c559
39cecec
d9ec2b0
3742af8
3acb223
084e6c7
cff6727
f1606cc
1140646
6d23628
f7a1ff4
974ad90
5ac143b
8ceaa3d
8d7159b
dbda7e9
342695e
0744b59
dad1b0b
bcf6664
921fecc
7920a45
c97630c
2724965
098ba86
49f9d6b
88c096b
37b4b4e
0c5e720
5840a1c
e1e6a8a
d7187b4
b648b10
f1454f2
e76bb06
b969e3f
0ff156a
309f9f0
e6d1301
4953a0c
d259127
1a16354
9aadf8f
e879978
f072943
c4d06ee
82eee6e
601119c
4f1ddb4
a6603cf
9b8d498
bd88cb2
abcb029
939456d
bf0aec2
0938b5e
94d9877
17ca088
2c2b38e
82ef52a
bc61288
7af15db
1e1752e
726e48c
21bd032
a9453d2
83b9074
e3a6c29
d34dcdb
72e9c2b
e291f91
795a568
9580649
74072f0
cf4dc34
f90965c
be75a36
780e9e7
fd1e27d
652557e
1da955d
5b809d5
b3fb72c
b5ca5ef
d5f5cba
82b97b8
d9b1733
ee6bef7
9e0b05c
f27e872
30dc02f
3d47210
fd56523
657ba78
a111532
6370194
e23ddb0
fc2b2a0
716ea1b
44df5dd
3a75fb0
84ebe2e
6b06732
6caef45
8eceac7
efd0705
82cf526
d133db7
651ff59
6209e6e
eaa0c7a
20477de
6195768
9cfb170
772691d
d71011b
991437d
a2253ec
d225aca
fc06639
1067a0c
ff4fadc
b976235
1a4345a
e5420c6
e183dcb
b2031f2
71b249c
c72984f
7da0c1b
797cb28
fad851a
a83b106
a424509
c69f2d5
887b6b4
c4eb362
55e8bf6
8b1bd7d
0b9d8b2
6a58538
8529284
a41edeb
0273b7b
5cbaedd
78472d9
fd0f7ec
d55be9e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When transforming a shortcode there is no
transform
method available, so no opportunity to create the innerBlocks as part of the transform process. The only workaround I could see for this was to add the data to a temporary attribute like with the file drag and drop, and process this in the editor when detected and then remove the attribute. Let me know if there is a way that I have overlooked to take the image ids from a shortcode and create the innerBlocks as part of the transform process instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same as above I guess, so there's already a solution in the image block itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case of the gallery short codes all we have is an id, and in the absence of a
transform
method there is no way to convert that id into a url - i did just try handing an id only to the Image block but it requires a url of some form - will take another look at this as well though in case I missed something.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right. It seems it's different than upload. though, I wonder if we should apply a similar technique:
If we detect an image block with an id but no url, triggers some effect in the image edit to get the remaining attributes right. We may need a "loading" state in the placeholder or something.
That said, the current approach is not that bad, it's just that attributes are meant to persist and this is changing that a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have had another look at this and the issue is that a shortcode transform provides no opportunity to return a block as there is no
transform
method as part of the transformation as noted here - #17758.If there was a
transform
method with shortcode transformations we could do as you suggested and just create the Image innerBlocks with an id only and let the Image block handle getting the url, etc. but currently the only way I can see to do this would be a two stage transform, eg. go fromshortcode
tocore/shortcode
block and then fromcore/shortcode
tocore/gallery
... but that seems like a messy user experience. Do you know why it is that shortcode transformations have no option to return a block from atransform
method?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ellatrix would know more here but I guess the response is that the schema was sufficient for the transforms we had to support so far
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reply, going through a bunch of notifications. Maybe there was never a need for it so far? I don't mind if we add a
transform
option.