RocketCommunicationsInc / astro

Astro UXDS is a collection of guidelines, patterns and components for designing space-based user interface applications.
https://astrouxds.com
Other
113 stars 25 forks source link

fix(rux-status) center icon within its space by default #1151

Closed kiley-mitti closed 1 year ago

kiley-mitti commented 1 year ago

Brief Description

I made a couple of small css tweaks to rux-status: 1) rux-status now displays inline-block rather than block 2) ::part[status] now has margin-inline(left and right) auto to center it in the space it takes up. Edit: Removed 2 in a subsequent commit after speaking with mark.

JIRA Link

ASTRO-6247

Related Issue

General Notes

This MIGHT be a major change or possibly minor. it is not just a patch since it changes the way the dimensions of this component work.. but it is easily changed back to the way it was before with a small css tweak.

Motivation and Context

This was a pain point for the TTC team and I think it's a legitimate one. In an application they needed to put status in .. basically its own table cell.

Example: https://codesandbox.io/s/agitated-hertz-7evbhb?file=/index.html

This is totally an easy fix, just add margin: 0 auto to part=status But.. this is SUCH a common use case that we even center the icon in our storybook examples. Storybook

And the only reason that the icon in he second set of examples (the one with all icons) centers without margin: auto is because display:flex is taking over the display:block on rux-status and forcing it to only be the width of its content.

Anyway I thought that such its such a simple change having a sample of it up for discussion might be good. So this draft is that sample. :)

Issues and Limitations

This is a breaking change. how severe is debatable but if we want to be cautious its probably major.. but its only major for people who, for whatever reason, WANT rux-status to take up all the space it is given while also leaving the icon to the left.. I can't say how many people that is but I don't know that its many.

If we remove change #1 (changing from block to inline-block) then it is either a patch or minor change since we're not changing the dimensions of the component.

Types of changes

Checklist

changeset-bot[bot] commented 1 year ago

āš ļø No Changeset found

Latest commit: 42bffc425ba459d7b3234affcce721c47e15bb1a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

netlify[bot] commented 1 year ago

Deploy Preview for astro-stencil ready!

Name Link
Latest commit 42bffc425ba459d7b3234affcce721c47e15bb1a
Latest deploy log https://app.netlify.com/sites/astro-stencil/deploys/647f6782f5ab010008d370f4
Deploy Preview https://deploy-preview-1151--astro-stencil.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 settings.

netlify[bot] commented 1 year ago

Deploy Preview for astro-preview ready!

Name Link
Latest commit 42bffc425ba459d7b3234affcce721c47e15bb1a
Latest deploy log https://app.netlify.com/sites/astro-preview/deploys/647f678217882b00081868ac
Deploy Preview https://deploy-preview-1151--astro-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 settings.

kiley-mitti commented 1 year ago

/update-snapshots

markacianfrani commented 1 year ago

going to close this for now due to breaking change philosophy. tickets have been updated so we can slot it into v8 since its not a huge change