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.

Sign up for GitHub

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

Jump to bottom

Creating Header-input class for dark header inputs #1506

Merged
merged 4 commits into from
Jul 20, 2021
Merged

Creating Header-input class for dark header inputs #1506

merged 4 commits into from
Jul 20, 2021

Conversation

jonrohan
Copy link
Member

@jonrohan jonrohan commented Jul 19, 2021 •
edited
Loading

Fixes #1481

I added a Header-input class to make search boxes in the header dark.

image

@changeset-bot
Copy link

changeset-bot bot commented Jul 19, 2021 •
edited
Loading

🦋 Changeset detected

Latest commit: 4715037

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/css Minor

Not sure what this means? Click here to learn what changesets are.

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

jonrohan
jonrohan commented Jul 19, 2021
View reviewed changes
src/header/header.scss
.Header-input {
color: var(--color-header-text);
background-color: var(--color-header-search-bg);
border: $border-width $border-style var(--color-header-search-border);
Copy link
Member Author

Choose a reason for hiding this comment

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

@simurai These were the variables used in dotcom, but I think they're slated to be deprecated. Should we add to https://github.com/primer/primitives/blob/d607f36c4c974017d6974382f9bb88d6418211b8/data/colors_v2/vars/component_dark.ts#L23-L26 a border and inset variable? We could also have something for buttons maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, there used to be the globalNav colors, but not sure where they were used.

I think this PR is good as is, but I guess we should move the header-search variables that are currently in "product" to "component". Now that the component supports the input.

Copy link
Contributor

Choose a reason for hiding this comment

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

Update: Moved in primer/primitives#181

@jonrohan jonrohan requested a review from simurai July 19, 2021 18:13
simurai
simurai approved these changes Jul 20, 2021
View reviewed changes
src/header/header.scss
.Header-input {
color: var(--color-header-text);
background-color: var(--color-header-search-bg);
border: $border-width $border-style var(--color-header-search-border);
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, there used to be the globalNav colors, but not sure where they were used.

I think this PR is good as is, but I guess we should move the header-search variables that are currently in "product" to "component". Now that the component supports the input.

@simurai simurai mentioned this pull request Jul 20, 2021
Merged
@simurai
Copy link
Contributor

simurai commented Jul 20, 2021

Ohh.. one more thing... does this input need a placeholder text color that is a bit dimmed? Currently on dotcom we use color: rgba(255, 255, 255, 0.75);.

@jonrohan jonrohan enabled auto-merge (squash) July 20, 2021 02:55
@jonrohan jonrohan disabled auto-merge July 20, 2021 02:55
@jonrohan jonrohan enabled auto-merge (squash) July 20, 2021 15:28
@jonrohan jonrohan merged commit 13cda22 into main Jul 20, 2021
@jonrohan jonrohan deleted the header-input branch July 20, 2021 15:29
@primer-css primer-css mentioned this pull request Jul 20, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simurai simurai simurai approved these changes

Assignees
No one assigned
Labels
minor release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add dark Header-input component
2 participants
@jonrohan @simurai