Skip to content
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

Autoblock image followed by cursive text into image-collage with 1 col #86

Merged
merged 2 commits into from
Oct 3, 2023

Conversation

bosschaert
Copy link

When an image is followed by cursive text, this combination with be auto-blocked into an image-collage with 1 column for an Image with a caption.

The caption can either follow the image directly:
Screenshot 2023-10-02 at 14 23 13

or it can follow the image on the next line:
Screenshot 2023-10-02 at 14 23 05

The result will be an image-collage block with the boxy-col-1 style
Screenshot 2023-10-02 at 14 25 15

Contributes to #85

Test URLs (see bottom for example):

@aem-code-sync
Copy link

aem-code-sync bot commented Oct 2, 2023

Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@aem-code-sync
Copy link

aem-code-sync bot commented Oct 2, 2023

Page Scores Audits Google
/career/boedi-hartono PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@bosschaert bosschaert requested a review from dnbute October 2, 2023 13:31
Copy link

@dnbute dnbute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there might be an edge-case,
https://issue-85--sunstar--hlxsites.hlx.page/_drafts/dbute/image-caption

For the first image, I pressed enter after the image in order to add the text
For the second image I pressed enter + shift after the image, which inserts a <br> element, and that seems to break the caption, I think it's good that you can control the autoblocking, but I'm not sure this was intended/if the authoring experience to "disable" auto-captioning is the best

Also, as a side-effect, the image seems to get smaller

Copy link

@dnbute dnbute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we maybe add something about autoblocking captions at the top of the picture, not only the bottom?
Screenshot 2023-10-03 at 09 56 22

https://www.sunstar.com/about/business-performance#FY2022

@bosschaert
Copy link
Author

Could we maybe add something about autoblocking captions at the top of the picture, not only the bottom?

Sure feel free to extend it!

@dnbute
Copy link

dnbute commented Oct 3, 2023

Could we maybe add something about autoblocking captions at the top of the picture, not only the bottom?

Sure feel free to extend it!

Sure, I'll do that as part of #87 after you merge

Copy link

@dnbute dnbute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All my concerns have been addressed, will extend this for my use-case after merge

@bosschaert bosschaert merged commit be78b70 into main Oct 3, 2023
4 checks passed
@bosschaert bosschaert deleted the issue-85 branch October 3, 2023 08:59
@sdmcraft sdmcraft added this to the Milestone-3 milestone Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants