Adjust borders on gallery to prevent overlap #27312
Conversation
karmatosed
added
the
[Block] Gallery
|
Size Change: -6.61 kB (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
|
Nice PR! I think you need to touch the borders on each individual gallery item instead of the gallery itself, becasue the outsetting of the border makes the placeholder differ from other placeholders. Before:
After:
If you adjust the box shadow on |
karmatosed
changed the title
|
Great feedback, thank you @jasmussen. I'm going to dig a bit more here as seems the images might hold the way forward for borders. There are some inconsistencies which as noted when more nuanced handling for child/parent blocks should help. For example I wonder if you should be able to see the outer and inner borders at the same time. I would suggest you shouldn't. Clicking on individual item to me should result in just this:
Whereas clicking on the container would show:
This feels beyond scope here potentially, but it certainly is something I'll follow through with other tickets, for example #27075. If there is a consistent way then styling these would be easier. |
|
I just updated this with a fix that focuses on the inner elements. Props to @ItsJonQ for some CSS guidance. After:
|
| @@ -38,13 +38,25 @@ figure.wp-block-gallery { | |||
| } | |||
|
|
|||
| figure.is-selected { | |||
| box-shadow: 0 0 0 $border-width $white, 0 0 0 3px var(--wp-admin-theme-color); | |||
|
|
|||
| border-radius: $radius-block-ui; | |||
There was a problem hiding this comment.
🎉 ! I noticed this during our 🍐 'ing session @karmatosed . The corners of the image seem to be peaking out from the focus ring. I think that would be resolved if we add this back in
There was a problem hiding this comment.
Unfortunately when I add that back in the overlap returns. There is the option of doing 'box-shadow: 0 0 0 $border-width $white;' but it then gives a slightly weird white outline. I'm very open to any ideas here as would love to get this smooth visually.
|
I just updated some iterations that take away the border-radius as it's not having an effect on this component. @jasmussen looping you in to check that this still keeps the component's visual consistency. With it in there was an artifact appearing as noted by @ItsJonQ.
It's my feeling that this is a good middle ground point to get for now around the overlap, as previously said longer term would be the iterations to this block planned. |
| img { | ||
| border-radius: $radius-block-ui; | ||
| &::before { | ||
| bottom: 0; |
There was a problem hiding this comment.
I know I'm being extremely nitpicky here, but I trust you'll forgive me.
I like to group related CSS together, so for example:
position: absolute;
top: 0;
right: 0;
bottom: 0;
left: 0;
— That is, the position is first, and then the directions shown one by one, clockwise, top right bottom left.
| box-shadow: 0 0 0 $border-width $white inset, 0 0 0 3px var(--wp-admin-theme-color) inset; | ||
| content: ""; | ||
| left: 0; | ||
| outline: 2px solid transparent; |
There was a problem hiding this comment.
Maybe add after this line, // Shown in Windows 10 high contrast mode. as a comment
|
@jasmussen thanks for that feedback, it was great and always keen to refine. I've made those changes and if you agree will then move this to merge. Thanks. |
|
I am just catching up on some old PRs sat. This seems like it can be merged without any issues still, so I am going to do just that right now. |
…bsolute positioned border. (#28992) #27312 introduced some absolute positioning to the gallery image border to fix some layout issues, but because this had a z-index specified it was sitting over the top of the caption which could no longer be selected. This PR just adds a z-index to caption also so it sits over the border. Co-authored-by: Glen Davies <glen.davies@a8c.com>
…bsolute positioned border. (#28992) #27312 introduced some absolute positioning to the gallery image border to fix some layout issues, but because this had a z-index specified it was sitting over the top of the caption which could no longer be selected. This PR just adds a z-index to caption also so it sits over the border. Co-authored-by: Glen Davies <glen.davies@a8c.com>
…bsolute positioned border. (#28992) #27312 introduced some absolute positioning to the gallery image border to fix some layout issues, but because this had a z-index specified it was sitting over the top of the caption which could no longer be selected. This PR just adds a z-index to caption also so it sits over the border. Co-authored-by: Glen Davies <glen.davies@a8c.com>
…bsolute positioned border. (#28992) #27312 introduced some absolute positioning to the gallery image border to fix some layout issues, but because this had a z-index specified it was sitting over the top of the caption which could no longer be selected. This PR just adds a z-index to caption also so it sits over the border. Co-authored-by: Glen Davies <glen.davies@a8c.com>
…bsolute positioned border. (#28992) #27312 introduced some absolute positioning to the gallery image border to fix some layout issues, but because this had a z-index specified it was sitting over the top of the caption which could no longer be selected. This PR just adds a z-index to caption also so it sits over the border. Co-authored-by: Glen Davies <glen.davies@a8c.com>
This iteration is hopefully a temporary workaround that having a better method for this block can solve in longer term. That said there is a current issue where the borders overlap on select.
Before:
In order to not touch the other elements that use this, I added to the block specific CSS. Props to @jasmussen for helping me learn more about this CSS and the potential iterations in future. It's my understanding whatever I am adding here will be changed later, so a workaround like this could be suitable.
After:
Feedback
I would love a code review as I admit, it got a little complex finding this specifically and I would love to know if there is a more elegant declaration. Similarly, whilst I took the method of treating borders to '-1px', perhaps alternatives could be done. I explored adding padding in the block, however that meant more visual impact.