Skip to content

fix(#1803): fix do/don't grids and component content grid#153

Merged
chrisolsen merged 1 commit into
GovAlta:alphafrom
bdfranck:fix-1803
May 8, 2024
Merged

fix(#1803): fix do/don't grids and component content grid#153
chrisolsen merged 1 commit into
GovAlta:alphafrom
bdfranck:fix-1803

Conversation

@bdfranck
Copy link
Copy Markdown
Collaborator

@bdfranck bdfranck commented May 3, 2024

This PR addresses the issues identified in GovAlta/ui-components#1803. It makes the following changes:

  • Reduces the minimum grid width of the "Do & Don't" columns from 50ch to 36ch
  • Swiches the ComponentContent grid to a single column when the table of contents disappears below 1230px width.
  • Adds a min-width of 0 to .component-content--content. This prevents wider content from growing the column width past the available space.
  • Fixes an undefined class name in ComponentContent

I determined that 36ch is the maximum width at 642px without overflowing the content area.

image

The header-related issues are captured in this other ticket: GovAlta/ui-components#1438

Checklist:

  • I have run a build locally.
  • I have tested the functionality.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 3, 2024

Deploy Preview for abgov-ui-component-docs ready!

Name Link
🔨 Latest commit 64ccda0
🔍 Latest deploy log https://app.netlify.com/sites/abgov-ui-component-docs/deploys/663bb4927df2b70008c3b666
😎 Deploy Preview https://deploy-preview-153--abgov-ui-component-docs.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.

@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented May 3, 2024

Here's how it looks in Alpha...
image

And here's how it looks in this PR:
image

@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented May 3, 2024

@twjeffery spotted an additional issue that is present on alpha: the content doesn't fill the full width when <624px wide.

image

Here's how it looks on the production site:
image

@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented May 3, 2024

I'm exploring what the above issues is...

@bdfranck bdfranck changed the title fix(#1803): reduce min width of do/don't grids fix(#1803): fix do/don't grids and component content grid May 3, 2024
@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented May 3, 2024

I think I found the source: The ComponentContent column grid template. It keeps 12rem of space for the table of contents even when it disappears below 1230px.

I've updated my PR to switch to one column below 1230px. I also fixed an issue causing an undefined className on component-content--content.

.component-content--container {
display: grid;
grid-template-columns: 1fr 12rem;
grid-gap: var(--goa-space-2xl);
}

@bdfranck bdfranck force-pushed the fix-1803 branch 2 times, most recently from 174735b to 9d139a5 Compare May 3, 2024 22:14
@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented May 3, 2024

@twjeffery also noticed that the table on the Error Text page was increasing the width of component-content--content. It looks like the table's horizontal scrolling isn't working.

image

I made the following updates to this PR to address the issue:

  • Add min-width: 0 to component-content--content so the column width stays within the container. (As suggested by CSS Tricks)
  • Wrap the table with <div style={{position: "relative", overflowX: "auto"}}> so it scrolls

I don't think the wrapper div is a great solution so I'm open to suggestions.

image

@ArakTaiRoth ArakTaiRoth marked this pull request as ready for review May 6, 2024 17:54
Comment thread src/components/component-content/component-content.css Outdated
Comment thread src/components/component-content/component-content.css Outdated
Comment thread src/routes/content/HelperText.tsx Outdated
@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented May 8, 2024

@chrisolsen Thanks for the feedback! I've updated my PR with the following changes:

  • Removed the scrolling wrapper div from the HelperText table
  • Reduced the indentation in component-content.css from 4 to 2 spaces

@chrisolsen chrisolsen merged commit 54a2c7a into GovAlta:alpha May 8, 2024
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.

[Bug]: Layout on design system website is breaking at certain viewport widths

2 participants