Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Fix dual-action button and dropdown component bugs #77

Merged
merged 3 commits into from
Oct 9, 2020

Conversation

ianrohde
Copy link
Contributor

@ianrohde ianrohde commented Oct 7, 2020

Summary of Changes

Adds:

  • vertical-align: middle added to all :root inline-flex components to correct alignment when there are multiple inline-flex elements placed inline.

Modifies:

  • Fix dual-action button and dropdown component bugs where alignment=left/right weren't responsive to parent width.

qa_req 0

additionally add `vertical-align: middle` :root `inline-flex` components
@ianrohde ianrohde self-assigned this Oct 7, 2020
@ianrohde ianrohde added bug Something isn't working core-design labels Oct 7, 2020
@ianrohde
Copy link
Contributor Author

ianrohde commented Oct 7, 2020

To bypass the compiled boilerplate, searching for components-web/src/components/ in the Files Changed tab should limit to the relative changes.

@rjmccluskey
Copy link
Contributor

LGTM! CR 🍻

@ianrohde
Copy link
Contributor Author

ianrohde commented Oct 9, 2020

dev_block 🌃 found a bug on storybook

@ianrohde
Copy link
Contributor Author

ianrohde commented Oct 9, 2020

un_dev_block 🧻

@rjmccluskey
Copy link
Contributor

CR 🍻

@@ -169,7 +169,7 @@

&::after {
content: "";
box-shadow: @shadow-3;
box-shadow: 2px 2px @space-2 rgba(17, 22, 26, 0.2);

Choose a reason for hiding this comment

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

Curious... what is the reasoning for it being okay to break from the pre-defined shadows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated because there's a 45 degree rotation on the pseudo-element on line 175, so the pre-defined shadows don't apply.

@hackalot805
Copy link

CR 💀 but maybe consider adding all of the generated files in a separate commit? Also, concerned about the lack of QA. Did you check these changes for browser compatibility? Might raise confidence if we could see before and after images too?

@jarstelfox
Copy link
Contributor

CR 🌵 @hackalot805 's didn't take for some reason

@ianrohde
Copy link
Contributor Author

ianrohde commented Oct 9, 2020

For QA, I have an issue open to do a full-pass of all components before we start using these in production: #17

I was spending too much time circling over browser issues repeatedly. Tony and I decided it would be more efficient to do it in one large pass.

@ianrohde ianrohde merged commit 0c3f87f into master Oct 9, 2020
@ianrohde ianrohde deleted the fix-dual-action-buttons branch October 9, 2020 23:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working core-design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants