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

Fix summary rendering when no valueList is supplied #3521

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

rmccar
Copy link
Contributor

@rmccar rmccar commented Mar 12, 2025

What is the context of this PR?

Have updated the summary macro to add a class that sets the width of the actions to be double width to take up the space that the valueList would take up. This fixes a misalignment when valueList is not provided. Also added tests for this and a missing test for the equivalent in actions.

Have also updated the example so that these scenarios are covered so if this ever happens again it should be picked up and have updated the documentation because either valueList or actions should be set to make the html correct from a semantic standpoint.

How to review this PR

  • Remove the setting of the valueList param from the example-summary-grouped file on main
  • See that on main it misaligns all the items in the summary
  • Compare this with this branch using the same config
  • See that the issue is fixed

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

Copy link

netlify bot commented Mar 12, 2025

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit eb24f2d
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/67d1b09fc14b760007219220
😎 Deploy Preview https://deploy-preview-3521--ons-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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.

1 participant