- Issue created by @joelpittet
- @joelpittet opened merge request.
- Status changed to Needs review
over 1 year ago 6:06pm 29 March 2023 - Status changed to RTBC
over 1 year ago 8:22pm 29 March 2023 - 🇺🇸United States smustgrave
Can confirm the issue described and that the MR fixes it. Same as the screenshots provided.
- Status changed to Needs work
over 1 year ago 1:47pm 31 March 2023 - 🇺🇸United States mherchel Gainesville, FL, US
The reason this was added is that the default
display: inline
UA styles for<img>
elements will leave space below the image in many circumstances (which can throw off vertical spacing).Because CKEditor labels the mode as inline, I think we should support that (even though I can't really think of a use case). But we shouldn't change the styles within the entire site.
Instead of modifying base.pcss.css, let's create an override within text-content.pcss.css that sets it to
display: revert
. Usingrevert
instead ofinline
will make it more explicit that we're trying to change back to the UA's default behavior. - Status changed to Needs review
over 1 year ago 3:05am 3 April 2023 - 🇮🇳India gauravvvv Delhi, India
Addressed #5, Instead modifying base.pcss.css file, I have overrided text-content.pcss.css file. please review
- Status changed to Needs work
over 1 year ago 2:09pm 3 April 2023 - 🇺🇸United States smustgrave
Believe it was discussed on slack that maybe a class gets added for inline. To cover backwards compatibility.
- Assigned to Muskan kumari
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 12:45pm 24 April 2023 - last update
over 1 year ago Patch Failed to Apply - Status changed to Needs work
over 1 year ago 10:56pm 24 April 2023 - 🇺🇸United States smustgrave
Still seems to be trying to address it purely with css. Think a class should be attempted.
- 🇮🇳India pradipmodh13 Ahmedabad
Hey @smustgrave,
Img is inline level element and in Olivero theme img tag css comes with 'display:block' css property from base.css file so I think we can remove display:block property.
For ref adding screenshot with display block and display inline property.
Can I remove property from base.css file and submit the patch ? - 🇮🇳India pradipmodh13 Ahmedabad
Removed display:block css from base file and it is better solution as img is inline level element.
For ref attached before and after screenshot.
Please review. - 🇮🇳India pradipmodh13 Ahmedabad
Hey @smustgrave,
If my patch is good then please change the status from needs work to needs review. - last update
over 1 year ago 30,322 pass - Status changed to Needs review
over 1 year ago 7:23pm 28 April 2023 - 🇨🇦Canada joelpittet Vancouver
@pradipmodh13, whenever you put a new patch up it's good if you put an interdiff here's the documentation on how to do that:
https://www.drupal.org/node/1488712 →And also feel free to set the status to Needs Review if you want people to look at the changes, that's it's intention. See the description of the statuses:
https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... → - Status changed to Needs work
over 1 year ago 3:41am 30 April 2023 - 🇺🇸United States smustgrave
Removing this CSS will most likely caused a visual regression on existing sites.
If the current fix solves the issues going forward thats great but think we need to consider existing sites too.
Which is why I suggested some sort of new class being added that the css can work with. That way the problem is solved and existing sites should be left alone.
- 🇳🇱Netherlands bskibinski Breda, The Netherlands
Just chiming in with some advice:
setting display: block on images only to fix the bottom "whitespace" issue is not the best way, images should be inline by default.A simple non-intrusive fix for the whitespace at the bottom of images is to just set "vertical-align: top" or any other statement except 'baseline' (I usually go for top).
Hope this helps.
- 🇺🇸United States ricksta
I strongly agree with the approach in #16 above. The other alignments mostly utilize adding a CSS class, too. Though I notice a
data-align="right"
in the CKEditor "source" becomesclass="align-right"
when a page is saved. So, I'm not sure what that attribute would be that would then become a CSSdisplay: inline
. I also agree with comment #17 that the default should be one where the adjacent content doesn't fall to the bottom. - Status changed to Needs review
4 months ago 7:40am 11 September 2024 hello,
i have tried the patch '3351145-in-line-from' and its working fine after remove
'display: block'
from the code and create a separate style for video working fine please check attachment.- Status changed to Needs work
4 months ago 8:49am 11 September 2024