Skip to content

Conversation

@saashapina
Copy link
Collaborator

@saashapina saashapina commented Apr 18, 2025

Summary of changes

  • Refactors and restyles Blockquote component
  • Adds tests
  • Adds a stylelint config to remove rule causing linting errors in base.css

*Jelly providing additional designs for attribution

Relevant Links

Test URLs:

Checklist

  • This PR has visual changes, and has been reviewed by a designer.
  • This PR has code changes, and our linters still pass.
  • This PR has new code, so new tests were added or updated, and they pass.

Validation

  1. Make sure all PR checks have passed.
  2. Pull down all related branches.
  3. Verify Blockquote align with mockups
  4. Verify all tests are passing

@aem-code-sync
Copy link

aem-code-sync bot commented Apr 18, 2025

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

@aem-code-sync
Copy link

aem-code-sync bot commented Apr 18, 2025

Page Scores Audits Google
📱 / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@catheraaine catheraaine changed the title feat: add Blockquote and Tests feat: Blockquote and Tests Apr 18, 2025
@catheraaine catheraaine changed the title feat: Blockquote and Tests feat: Blockquote Apr 18, 2025
@aem-code-sync aem-code-sync bot temporarily deployed to ADB-173-blockquote April 18, 2025 15:23 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to ADB-173-blockquote April 18, 2025 16:11 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to ADB-173-blockquote April 18, 2025 16:13 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to ADB-173-blockquote April 19, 2025 04:05 Inactive
@arnest00 arnest00 self-requested a review April 19, 2025 05:33
@@ -1,20 +1,30 @@
export default async function decorate(block) {
const [quotation, attribution] = [...block.children].map((c) => c.firstElementChild);
const [quotation, attribution, attributionUrl] = [...block.children].map((child) => child.firstElementChild);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we go another step further and remove the nested divs such that the final output of this block is:

<blockquote>
  <p class="quote__quotation">...</p>
  <p class="quote__attribution">
    <cite>...</cite>
  </p>
</blockquote>

I moved around the structure a bit there - we can nest a <cite> within a <p> tag, but not the other way around. Let's also update the CSS class names to BEM-style naming.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great call

}
margin-block-start: 0.5rem;

& p {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this utility class existed when you wrote this, but for the attribution text, we can give it the .util-detail-s class in quote.js!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perfect!

@aem-code-sync aem-code-sync bot temporarily deployed to ADB-173-blockquote April 21, 2025 15:49 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to ADB-173-blockquote April 21, 2025 19:10 Inactive
Copy link
Collaborator

@arnest00 arnest00 left a comment

Choose a reason for hiding this comment

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

🚀

@saashapina saashapina merged commit 138f872 into main Apr 21, 2025
5 checks passed
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.

4 participants