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

Update button and form styles #1482

Merged
merged 48 commits into from Jan 12, 2021
Merged

Conversation

skoolbus39
Copy link
Member

@skoolbus39 skoolbus39 commented Nov 25, 2020

Don't merge until January. Safe to merge (pending review).

skoolbus39 and others added 29 commits August 18, 2020 21:50
Fix variable used in background-color mixin (5.2 dark mode preview)
@skoolbus39 skoolbus39 added the CSS label Nov 25, 2020
@skoolbus39 skoolbus39 changed the title WIP: Update button and form styles Update button and form styles Jan 8, 2021
@jsturek
Copy link
Contributor

jsturek commented Jan 12, 2021

Screen Shot 2021-01-12 at 1 15 40 PM

Probably an existing issue but buttons do not stack well.

.unl .dcf-btn-tertiary:active {
color: $color-active-light-mode;
.unl .dcf-btn:focus {
outline: 3px solid transparent; // https://sarahmhigley.com/writing/whcm-quick-tips/
Copy link
Contributor

Choose a reason for hiding this comment

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

Need this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd prefer to keep this one to provide context.

.unl .dcf-input-select:hover, // TODO: deprecate?
.unl .dcf-form select:hover {
box-shadow: 0 0 $length-em-3 fade-out($border-color-input-hover-light-mode,.25);
// box-shadow: 0 0 $length-em-3 fade-out($border-color-input-hover-light-mode,.25);
Copy link
Contributor

Choose a reason for hiding this comment

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

Need this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

h1 {
margin: .67em 0;
}
// h1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove?

Copy link
Member Author

Choose a reason for hiding this comment

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

The critical CSS is manually constructed from existing stylesheets. I commented rather than removed code so I could effectively toggle as many high-priority styles as possible until we got to ~13KB. Revisit in a separate issue?

template {
display: none;
}
// dialog:not([open]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above comment

@jsturek
Copy link
Contributor

jsturek commented Jan 12, 2021

See comments. Note: css comments need to refer to associated scss.

@skoolbus39
Copy link
Member Author

Button stacking is an existing issue. I've never found a one-size-fits-all fix for this issue, but we can revisit.

@sonarcloud
Copy link

sonarcloud bot commented Jan 12, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jsturek jsturek self-requested a review January 12, 2021 19:41
Copy link
Contributor

@jsturek jsturek left a comment

Choose a reason for hiding this comment

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

Looks good

@skoolbus39 skoolbus39 merged commit c2ddeb7 into unl:5.2 Jan 12, 2021
@skoolbus39 skoolbus39 deleted the 5.2-new-buttons-forms branch February 14, 2022 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants