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

feat(masthead-v2): Minimize CMApp on megamenu open #11085

Merged
merged 9 commits into from
Nov 13, 2023
Merged

feat(masthead-v2): Minimize CMApp on megamenu open #11085

merged 9 commits into from
Nov 13, 2023

Conversation

jkaeser
Copy link
Member

@jkaeser jkaeser commented Nov 1, 2023 •
edited by m4olivei
Loading

Related Ticket(s)

Jira

Description

Using the Contact Module's init method as a workaround to minimize it has the potential to completely break its opening & closing. In a discussion with folks from that team, we've agreed to call the minimize method, which is new since our workaround was put in place.

Changelog

New

  • Minimizes the Contact Module application when opening a megamenu.

Removed

  • Clicking the contact link in the masthead when the Contact Module is open no longer closes the module.

@jkaeser
feat(masthead-v2): contact link should not affect CM_APP
3ddb102 
This has the potential to clobber the global app instance,
preventing it from opening or closing altogether.
@jkaeser
feat(masthead-v2): minimize CMApp on megamenu open
4fd0340
@jkaeser jkaeser added package: web components Work necessary for the IBM.com Library web components package 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 Nov 1, 2023
@jkaeser
Copy link
Member Author

jkaeser commented Nov 1, 2023

Note: This doesn't fully work yet. Calling the minimize method somehow seems to trigger the Lit lifecycle, resulting in megamenus closing rapidly after opening if the Contact Module was previously opened.

@andy-blum
Copy link
Contributor

Calling the minimize method somehow seems to trigger the Lit lifecycle

I wonder if the minimize method of the CM_APP is moving focus somewhere

@andy-blum
Copy link
Contributor

Confirmed. Until this behavior is fixed from the contact module, this PR is unmergable.

screenshot_2023-11-01_at_4 49 00___pm

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Nov 1, 2023 •
edited
Loading

Deploy preview created for package Web Components (React wrapper):
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11085/index.html

Built with commit: 91ed85ca152d1b8b207e9a901d093ceed4a82a89

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Nov 1, 2023 •
edited
Loading

Deploy preview created for package Web Components HTML Test Application:
https://ibmdotcom-web-components-html-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11085/index.html

Built with commit: 91ed85ca152d1b8b207e9a901d093ceed4a82a89

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Nov 1, 2023 •
edited
Loading

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11085/index.html

Built with commit: 91ed85ca152d1b8b207e9a901d093ceed4a82a89

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Nov 1, 2023 •
edited
Loading

Deploy preview created for package React:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/11085/index.html

Built with commit: 91ed85ca152d1b8b207e9a901d093ceed4a82a89

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Nov 1, 2023 •
edited
Loading

Deploy preview created for package Carbon Web Components:
https://carbon-web-components.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11085/index.html

Built with commit: 91ed85ca152d1b8b207e9a901d093ceed4a82a89

@andy-blum andy-blum requested a review from m4olivei November 6, 2023 20:46
@andy-blum andy-blum marked this pull request as ready for review November 6, 2023 20:46
@andy-blum andy-blum requested a review from a team as a code owner November 6, 2023 20:46
@andy-blum andy-blum requested review from ariellalgilmore and sangeethababu9223 and removed request for a team November 6, 2023 20:46
@andy-blum andy-blum mentioned this pull request Nov 6, 2023
Closed
2 tasks
m4olivei
m4olivei previously requested changes Nov 6, 2023
View reviewed changes
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.

I think we can simplify the event handling by using the @HostListener decorator with the document: prefix for the event name. See comment for details.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2023 •
edited
Loading

DCO Assistant Lite bot All Contributors have signed the CLA.

@andy-blum andy-blum force-pushed the feat/masthead-v2-cmapp-minimize branch from a142cdf to 21177e5 Compare November 8, 2023 14:27
@ibmdotcom-bot
Copy link
Contributor

Deploy preview created for package NextJS Test Application:
https://ibmdotcom-nextjs-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11085/index.html

Built with commit: 91ed85ca152d1b8b207e9a901d093ceed4a82a89

@andy-blum
Copy link
Contributor

Verified with @jkaeser and @m4olivei on a call since testing requires network request modifications.

@andy-blum andy-blum added Ready to merge Label for the pull requests that are ready to merge and removed DO NOT MERGE labels Nov 13, 2023
@andy-blum andy-blum dismissed m4olivei’s stale review November 13, 2023 18:02

changed as requested

@andy-blum andy-blum merged commit c4a06f3 into carbon-design-system:feat/masthead-v2-dev Nov 13, 2023
22 of 23 checks passed
m4olivei
m4olivei reviewed Nov 13, 2023
View reviewed changes
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.

LGTM! Glad the arrow function approach worked.

jkaeser added a commit to jkaeser/carbon-for-ibm-dotcom that referenced this pull request Dec 1, 2023
@jkaeser @andy-blum
feat(masthead-v2): minimize CMApp on megamenu open (carbon-design-sys…
b040c9c 
…tem#11085)

* feat(masthead-v2): contact link should not affect CM_APP

This has the potential to clobber the global app instance,
preventing it from opening or closing altogether.

* feat(masthead-v2): minimize CMApp on megamenu open

* feat(masthead-v2): close chat module on L1 dropdown open

* feat(masthead-v2): toggle masthead-contact label on cm-app state change

* feat(masthead-contact): update cm-app event listeners

* fix(masthead-contact): revert to hostlistener usage

* fix(masthead-contact): remove console logs

---------

Co-authored-by: Andy Blum <[email protected]>
@m4olivei m4olivei mentioned this pull request Jan 2, 2024
Merged
kodiakhq bot pushed a commit that referenced this pull request Jan 8, 2024
@jkaeser
chore(masthead): integrate enhancements from alpha branch (#11183)
eec379e 
### 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
@jkaeser @kennylam
chore(masthead): integrate enhancements from alpha branch (carbon-des…
297fca5 
…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
Reviewers

@m4olivei m4olivei m4olivei left review comments

@andy-blum andy-blum andy-blum approved these changes

@ariellalgilmore ariellalgilmore Awaiting requested review from ariellalgilmore ariellalgilmore was automatically assigned from carbon-design-system/carbon-for-ibm-com-members

@sangeethababu9223 sangeethababu9223 Awaiting requested review from sangeethababu9223 sangeethababu9223 was automatically assigned from carbon-design-system/carbon-for-ibm-com-members

Assignees
No one assigned
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 package: web components Work necessary for the IBM.com Library web components package Ready to merge Label for the pull requests that are ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkaeser @andy-blum @ibmdotcom-bot @m4olivei