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

feat(masthead-v2): add masthead to document flow #10998

Conversation

jkaeser
Copy link
Member

@jkaeser jkaeser commented Oct 3, 2023

Related Ticket(s)

Description

The visible parts of the masthead are position: fixed;, which prevents them from being in the document flow. This forces adopters to manually add spacing to their page content so the masthead doesn't overlap. We provide the --dds-sticky-header-height variable via the StickyHeader utility as a value to use, but this changes as the user scrolls, creating a potentially unwanted parallax effect.

parallax

To overcome this, we force the masthead's container into the document flow by giving it a height equal to its visible parts' height. This avoids the parallax effect, and adopters should only need to add the spacing they want between their content and the masthead, rather than solving for the masthead's height in some way.

Testing

Visit deploy preview Stories for the masthead with and without the L1. In each, use the browser dev tools to inspect the element and find the <dds-masthead-container> or <dds-masthead-composite> (they are mostly analogous). It should have a CSS height set on it equal to the <dds-masthead> element's height.

Changelog

Changed

  • Puts the masthead into the document flow so it naturally takes up space equivalent to its height.

@jkaeser jkaeser added adopter: AEM used when component or pattern will be used by this adopter owner: Innovation Team used when the engineering work will be done by Hybrid Cloud with DDS engineers as consultants Masthead L0/L1 v2 used when component or pattern is part of the Mashead V2 update effort labels Oct 3, 2023
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 3, 2023

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 3, 2023

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 3, 2023

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 3, 2023

@jkaeser jkaeser marked this pull request as ready for review October 3, 2023 18:48
@jkaeser jkaeser requested a review from a team as a code owner October 3, 2023 18:48
@jkaeser jkaeser requested review from kennylam, IgnacioBecerra, m4olivei, andy-blum, bruno-amorim and marcelojcs and removed request for a team October 3, 2023 18:48
Copy link
Contributor

@andy-blum andy-blum left a comment

Choose a reason for hiding this comment

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

This looks pretty good - suggesting a change from mutationObserver to resizeObserver

Copy link
Contributor

@m4olivei m4olivei left a comment

Choose a reason for hiding this comment

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

Looks great! Just small comment about updating a comment.

@jkaeser jkaeser requested a review from andy-blum October 4, 2023 16:28
@jkaeser jkaeser force-pushed the feat/set-masthead-height branch from 9ff1d18 to ec719e5 Compare October 10, 2023 19:14
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Oct 10, 2023

@andy-blum andy-blum self-requested a review October 12, 2023 18:41
Copy link
Member

@kennylam kennylam left a comment

Choose a reason for hiding this comment

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

LGTM thanks @jkaeser!

@kennylam kennylam added the Ready to merge Label for the pull requests that are ready to merge label Nov 6, 2023
@kennylam kennylam merged commit e1debd8 into carbon-design-system:feat/masthead-v2-dev Nov 7, 2023
20 of 23 checks passed
@jkaeser jkaeser deleted the feat/set-masthead-height branch November 7, 2023 16:55
jkaeser added a commit to jkaeser/carbon-for-ibm-dotcom that referenced this pull request Dec 1, 2023
…m#10998)

* feat(masthead-v2): put container in document flow

* feat(masthead-v2): format code

* feat(masthead-v2): update height on resize

* feat(masthead-v2): accurately describe observer

* feat(masthead-v2): debounce resize observer callback

* feat(masthead-v2): execute callback once after successive resizes

---------

Co-authored-by: Andy Blum <[email protected]>
kodiakhq bot pushed a commit that referenced this pull request Jan 8, 2024
### Related Ticket(s)

none

### Description

We made some enhancements to the alpha v2 masthead after we'd integrated its branch into the main C4IBM v2 branch. This PR ports those enhancements over.

- **fix(masthead-v2): use more explicit selector in dropdown toggle** (911f802) ([Original PR](#11084))
- **feat(masthead-v2): add masthead to document flow** (e1debd8) ([Original PR](#10998))
- **feat(masthead-v2): Minimize CMApp on megamenu open** (c4a06f3) ([Original PR](#11085))
- **fix(masthead-v2): do not fail if object data does not exist** (c9609f7) ([Original PR](#11205))
- **fix(masthead-v2): ensure L1 dropdown targets exist** (fc65086) ([Original PR](#11254))
- **fix(masthead-v2): provide accessible nav label** (f183d25) ([Original PR](#11269))
- **fix(sticky-header): prevent negative overscroll from hiding stuck elements** (51015ed) ([Original PR](#11266))

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
kennylam pushed a commit to kennylam/carbon-for-ibm-dotcom that referenced this pull request Mar 20, 2024
…ign-system#11183)

none

We made some enhancements to the alpha v2 masthead after we'd integrated its branch into the main C4IBM v2 branch. This PR ports those enhancements over.

- **fix(masthead-v2): use more explicit selector in dropdown toggle** (911f802) ([Original PR](carbon-design-system#11084))
- **feat(masthead-v2): add masthead to document flow** (e1debd8) ([Original PR](carbon-design-system#10998))
- **feat(masthead-v2): Minimize CMApp on megamenu open** (c4a06f3) ([Original PR](carbon-design-system#11085))
- **fix(masthead-v2): do not fail if object data does not exist** (c9609f7) ([Original PR](carbon-design-system#11205))
- **fix(masthead-v2): ensure L1 dropdown targets exist** (fc65086) ([Original PR](carbon-design-system#11254))
- **fix(masthead-v2): provide accessible nav label** (f183d25) ([Original PR](carbon-design-system#11269))
- **fix(sticky-header): prevent negative overscroll from hiding stuck elements** (51015ed) ([Original PR](carbon-design-system#11266))

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adopter: AEM used when component or pattern will be used by this adopter Masthead L0/L1 v2 used when component or pattern is part of the Mashead V2 update effort owner: Innovation Team used when the engineering work will be done by Hybrid Cloud with DDS engineers as consultants Ready to merge Label for the pull requests that are ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants