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 header component #95

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Update header component #95

merged 1 commit into from
Jan 27, 2025

Conversation

thatbudakguy
Copy link
Member

@thatbudakguy thatbudakguy commented Jan 21, 2025

This updates the designs to match the v.2 header component in Figma.

The templates were updated to make the content as close as possible to what's currently in Figma.

Closes #92

styles/header.css Outdated Show resolved Hide resolved
styles/header.css Outdated Show resolved Hide resolved
styles/palette.css Outdated Show resolved Hide resolved
styles/links.css Outdated Show resolved Hide resolved
styles/header.css Show resolved Hide resolved
* an <li> because they would be counted as items in the list. Repositioning of
* other elements during opening of the menu makes them jump around, so we also
* have to disable animation of opening/closing the nav. */
.navbar hr {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does this occur?

Copy link
Member Author

Choose a reason for hiding this comment

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

<hr class="d-md-none text-white vw-100 my-0" />
and
<hr class="d-md-none text-white vw-100 my-0" />

styles/header.css Outdated Show resolved Hide resolved
Copy link
Contributor

@jcoyne jcoyne left a comment

Choose a reason for hiding this comment

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

This is a lot to digest at once. Is there any way to break it up into smaller pieces of work?

@thatbudakguy
Copy link
Member Author

Broke off the changes that add the new variants so that this PR is only the changes to the existing dark variants for easier review.

@thatbudakguy thatbudakguy requested a review from jcoyne January 22, 2025 18:16
@thatbudakguy thatbudakguy changed the title Refactor headers; add new variants Update header component Jan 22, 2025
@thatbudakguy thatbudakguy temporarily deployed to github-pages-preview January 23, 2025 16:43 — with GitHub Actions Inactive
@thatbudakguy thatbudakguy temporarily deployed to github-pages-preview January 23, 2025 16:45 — with GitHub Actions Inactive
styles/header.css Show resolved Hide resolved
Copy link

A preview of this branch is available at https://sul-dlss.github.io/component-library/preview-95.

@dnoneill
Copy link
Contributor

The opening of the mobile menu is kind of choppy

@thatbudakguy
Copy link
Member Author

It's supposed to just be immediate (no animation at all). Is that what you see?

@dnoneill
Copy link
Contributor

Yeah, currently there is animation: https://sul-dlss.github.io/component-library/header/

@thatbudakguy
Copy link
Member Author

Right. This PR turns it off:

/* HRs in the navbar have to be positioned vertically relative to the nav lists,
* but horizontally relative to the entire container. They aren't valid as
* children of a <ul>, so they have to be siblings, and they can't be inside
* an <li> because they would be counted as items in the list. Repositioning of
* other elements during opening of the menu makes them jump around, so we also
* have to disable animation of opening/closing the nav. */

@thatbudakguy thatbudakguy requested a review from jcoyne January 27, 2025 17:45
@jcoyne jcoyne merged commit 2cfad24 into main Jan 27, 2025
10 of 11 checks passed
@jcoyne jcoyne deleted the header-fixes branch January 27, 2025 18:17
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.

Dropdowns in header menu
3 participants