feat(cloud-masthead): add Cloud-specific container and composite components #5593

huntermacd · 2021-03-24T17:27:16Z

Related Ticket(s)

https://jsw.ibm.com/browse/HC-1681

Description

This diff adds various Cloud-specific versions of existing components. This should be kept as a draft since there is a lot more work to do, but I need some eyes from the team on the work so far for guidance.

Changelog

New

cloud-masthead-container component
cloud-masthead-composite component
cloud-masthead-profile component

Changed

add Storybook story for Cloud variant masthead
added selector to masthead.scss to include Cloud-specific masthead

ibmdotcom-bot · 2021-03-24T17:47:25Z

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/5593/index.html

Built with commit: 7c2066c00d4ae455a62674e2b269f9bc78eb5ed9

ibmdotcom-bot · 2021-03-24T17:51:40Z

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

Built with commit: 7c2066c00d4ae455a62674e2b269f9bc78eb5ed9

ibmdotcom-bot · 2021-03-24T18:06:14Z

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

Built with commit: 7c2066c00d4ae455a62674e2b269f9bc78eb5ed9

annawen1 · 2021-03-24T18:52:50Z

packages/web-components/src/components/masthead/__stories__/masthead.stories.ts

+      ?has-profile="${hasProfile}"
+      ?has-search="${hasSearch}"
+      .unauthenticatedProfileItems="${ifNonNull(unauthenticatedProfileItems)}"
+      data-endpoint="/common/carbon-for-ibm-dotcom/translations/cloud-masthead"


was double checking why the endpoint was resulting in a 404 - turns out I forgot we need to add /jsononly to the path. We should be able to get the data using /common/carbon-for-ibm-dotcom/translations/cloud-masthead/jsononly

@annawen1 This is good to know. Thanks for pointing this out. Also, since we're using the JSON-only directory, I'm guessing we don't need the .js within the dotcom translation repo? https://github.ibm.com/webstandards/ibm-dotcom-translations/blob/master/translations/cloud-masthead/usen.js

I just wanted to bring this up as this will play in our translation content population workflow and if we're only using the JSON files that would help come down in the formatting of the .js versions. Let me know what you think.

Yeah we don't need the .js file for the translations, I think we initially kept it for consistency when copying over the current ibm.com translations folder structure? Not sure if removing it will cause any issues. @jeffchew what do you think?

@annawen1 Thanks! @jeffchew Let me know if we need to account for the .js files within the Cloud Masthead translation directory.

packages/web-components/src/components/masthead/cloud/cloud-masthead-container.ts

packages/web-components/src/components/masthead/__stories__/masthead.stories.ts

packages/web-components/src/components/masthead/cloud/cloud-masthead-composite.ts

packages/web-components/src/components/masthead/__stories__/masthead.stories.ts

packages/web-components/src/components/masthead/cloud/cloud-masthead-composite.ts

proeung · 2021-03-24T20:48:15Z

packages/web-components/src/components/masthead/cloud/cloud-masthead-composite.ts

+            ${profileItems?.map(
+              ({ title, url }) =>
+                html`
+                  <dds-masthead-profile-item href="${ifNonNull(url)}">${title}</dds-masthead-profile-item>


As part of the new Cloud masthead work, we need to expose a CTA button component that links out to the console. This is a new feature that we'll need to account for within the authenticated state of the dds-cloud-masthead-profile component (see specs - https://ibm.ent.box.com/file/786859527229).

As for the data/translation, we'll need to add this to our JSON file as well. @annawen1 @jeffchew Let me know if there are fields within the existing data structure that we can reuse or if this is something new that we need to add. https://1.www.s81c.com/common/carbon-for-ibm-dotcom/translations/cloud-masthead/jsononly/usen.json

hmm don't see any existing fields we can utilize for the console button so I think we need to add it. Possibly in

"masthead": { "search": { "btnSearchClosed": "Open IBM search field", "btnSearchOpen": "Search all of IBM", "btnClose": "Close IBM search field", "placeHolderText": "Search all of IBM" }, --> "not sure key name bc I'm not good with naming": "Console"

@annawen1 Looking at the existing structure, I think it might be good to add this button within the [profileMenu][signedin] array, as this button is intended to show up in the authenticated state (see attached ). This will be managed outside of the [profileMenu][signedin][links], which is being used to display the user drop-down menu. Let me know what you think.

@huntermacd let me know what you think as well.

yep, that looks good! Just note we need to edit the type files in the services store package! Let me know if you need help with that.

@annawen1 Great! I'll work on opening a PR to add this new ctaButton array. As for accounting for these types in the services store package, perhaps this is something you can help us add once the data is in place?

Here's the PR to adjust the data structure based on our conversation here - https://github.ibm.com/webstandards/ibm-dotcom-translations/pull/295.

proeung

@huntermacd Overall, this is a great first draft of the work, and thanks for putting this PR together. @annawen1 and I left some feedback. Let me if you have any questions/concerns.

asudoh

Thanks @huntermacd!

@jeffchew Just double-checking; Looks that we are maintaining these components even though it's cloud-BU-specific. Is it correct? Just wanted to see if we decided that PAL route didn't make sense.

packages/web-components/src/components/masthead/cloud/cloud-masthead-profile.ts

huntermacd · 2021-03-30T15:50:07Z

cc @annawen1 @asudoh @jeffchew

I could use some assistance wrapping these components behind a feature flag, being that they're interconnected. My latest commit 70085a3 shows my progress on doing that. Currently it's broken. I may have a circular import happening.

Also, regarding the cookie check, this needs to happen on each load of the component. I'm not certain that there's a need to put this value into the store. The goal is to check the cookie for determining which version of the Cloud Masthead Profile to show, either "authenticated" or "anonymous". Putting this functionality into an importable service, a la the ProfileAPI, also seems like overkill when it boils down to a single, simple function. Please let me know your thoughts on the matter.

annawen1 · 2021-03-30T17:44:01Z

cc @annawen1 @asudoh @jeffchew

I could use some assistance wrapping these components behind a feature flag, being that they're interconnected. My latest commit 70085a3 shows my progress on doing that. Currently it's broken. I may have a circular import happening.

Also, regarding the cookie check, this needs to happen on each load of the component. I'm not certain that there's a need to put this value into the store. The goal is to check the cookie for determining which version of the Cloud Masthead Profile to show, either "authenticated" or "anonymous". Putting this functionality into an importable service, a la the ProfileAPI, also seems like overkill when it boils down to a single, simple function. Please let me know your thoughts on the matter.

@asudoh wonder what your thoughts are on where the cookie fetching logic lives?

asudoh · 2021-03-31T12:34:50Z

@annawen1 Thank you for asking. If the team thinks maintaining the login status in @carbon/ibmdotcom-services-store, they can manually change userStatus property in <dds-masthead-composite> or one in <dds-masthead-container>. In other words, cookie handling code should not be baked in to <dds-masthead-*>.

annawen1 · 2021-03-31T15:04:58Z

@annawen1 Thank you for asking. If the team thinks maintaining the login status in @carbon/ibmdotcom-services-store, they can manually change userStatus property in <dds-masthead-composite> or one in <dds-masthead-container>. In other words, cookie handling code should not be baked in to <dds-masthead-*>.

Thanks @asudoh! @huntermacd, based on Akira's comment I think it may be good to fetch the cookie data within dds-cloud-masthead-container and set cookie value to userStatus. This way we won't have to put the logic in the services-store package and we won't be baking it into dds-masthead-* components

ibmdotcom-bot · 2021-04-02T12:58:36Z

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

Built with commit: 7c2066c00d4ae455a62674e2b269f9bc78eb5ed9

proeung

@huntermacd Looks great! I'm now able to see the CTA button for the anonymous and authenticated states!

@annawen1 @jeffchew I think this PR is good to merge down to the main branch and available for view via the experimental environment. What do you both think?

annawen1 · 2021-04-08T21:36:05Z

@huntermacd looks good! 🎉

One thing - I do see it's failing on one of the tests, can you take a look?

proeung · 2021-04-09T17:43:45Z

Looks like all of the CI checks are cleared! @annawen1 Are we good to merge this PR down?

annawen1 · 2021-04-09T18:29:16Z

Looks like all of the CI checks are cleared! @annawen1 Are we good to merge this PR down?

Everything looks good, one thing though sorry! @jeffchew you mentioned about the js-cookie dependency and if it would make more sense to have the cookie fetching logic as part of the utilities package where we already have js-cookie?

We have a similar util called ipcInfoCookie.js there.

jeffchew · 2021-04-09T18:37:59Z

Looks like all of the CI checks are cleared! @annawen1 Are we good to merge this PR down?

Everything looks good, one thing though sorry! @jeffchew you mentioned about the js-cookie dependency and if it would make more sense to have the cookie fetching logic as part of the utilities package where we already have js-cookie?

We have a similar util called ipcInfoCookie.js there.

That's right, and sorry that I'm catching up with this PR now. What caught my attention was the js-cookie dependency getting added to web-components, where we have separated out the cookie setting and fetching logic (to @annawen1 's point) in our utilities package. It may be nice to also have it there so that the handling of that is centralized.

@asudoh to your question about PAL specific things, I agree that that would be the ultimate goal. This is a first iteration of the Cloud masthead which is why we have suggested putting behind a feature flag. At least the way I'm thinking of, it would be nice to be able to ultimately abstract some of the things happening in this variation of the Masthead so that other teams/BUs can ultimately take advantage of some of the things here, and Cloud would also ultimately be using this abstracted version for their needs too. But for now it can be safely iterated behind the flag while also getting something that can hit the glass for the Cloud team.

proeung · 2021-04-09T19:21:26Z

@jeffchew Thanks for the response! Sounds to me like we have a good direction on how to manage the cookie settings and fetching logic within the existing @carbon/ibmdotcom-utilities package for future implementation. In the meantime, I think we can proceed with the feature flag and account for the abstraction at a later time. Let me know if you'd like me to create a GH issue for this feature.

jeffchew

@proeung yes, would typically agree that if we're behind a feature flag, we can safely continue to iterate behind it. However, I think the added dependency in the package itself, regardless if the intention is to move to another package later on, is something we should try to avoid.

I think if at minimum we can move the cookie library dependency out of the web-components package, I think that can be safe to then merge this in.

proeung · 2021-04-12T14:42:27Z

@jeffchew Sounds good! I'll create a ticket for us to track the work for moving the js-cookie dependency and the fetching logic to the existing @carbon/ibmdotcom-utilities package. In the meantime, I think this recent commit (33cf590) that @huntermacd made should allow this PR to move forward with merging.

@shixiedesign I don't believe this PR requires a design QA at the moment. We're working on getting the baseline changes for the new Cloud masthead merged down so that we can add in the other design enchantments. Also, the changes in this PR are behind a feature flag. We'll conduct a proper design QA review once we have all of the elements assembled that you, Lila Title, and others can take a look at.

huntermacd · 2021-04-12T14:46:47Z

With the removal of the js-cookie dependency in the web components package, we're getting an import/no-extraneous-dependencies eslint error which is causing CI to fail. I'm not terribly familiar with Lerna or Yarn's workspaces feature, but I'm assuming we're setup to share dependencies in exactly this way. Questions: am I correct in that assumption? And if so, what's the preferred method for getting around this eslint error in cases like this? @jeffchew

annawen1 · 2021-04-12T17:34:02Z

Pushed changes to move the cookie fetching logic in the utilities package!

proeung · 2021-04-12T18:13:21Z

@annawen1 Awesome job moving the js-cookie fetching logic into the @carbon/ibmdotcom-utilities package! You've saved us a lot of time with creating the ticket and implementing this work, so thank you so much! 🥇

@jeffchew With the latest commit (864d893) that @annawen1 pushed up, I think we're good for another code review and I think we can safely merge this PR down. What do you think?

annawen1

LGTM!

…m into feat/5305 * 'feat/5305' of github.com:iwhitcomb/carbon-for-ibm-dotcom: chore(carbon): upgrade to Carbon v10.31.0 (carbon-design-system#5760) feat(cloud-masthead): add Cloud-specific container and composite components (carbon-design-system#5593) fix(dotcom-shell): add scroll eventListener (carbon-design-system#5747) fix(card-pictogram): fixed visual bugs & storybook refinements (carbon-design-system#5736)

huntermacd added 2 commits March 23, 2021 14:04

feat(cloud-masthead): WIP stub Cloud masthead components

06934f1

feat(cloud-masthead): wip add storybook variant

6b415c6

huntermacd changed the title ~~Cloud masthead variant~~ feat(cloud-masthead): add Cloud-specific container and composite components Mar 24, 2021

annawen1 reviewed Mar 24, 2021

View reviewed changes

packages/web-components/src/components/masthead/cloud/cloud-masthead-container.ts Outdated Show resolved Hide resolved

proeung reviewed Mar 24, 2021

View reviewed changes

packages/web-components/src/components/masthead/__stories__/masthead.stories.ts Outdated Show resolved Hide resolved

proeung reviewed Mar 24, 2021

View reviewed changes

packages/web-components/src/components/masthead/cloud/cloud-masthead-composite.ts Outdated Show resolved Hide resolved

proeung reviewed Mar 24, 2021

View reviewed changes

packages/web-components/src/components/masthead/__stories__/masthead.stories.ts Outdated Show resolved Hide resolved

annawen1 reviewed Mar 24, 2021

View reviewed changes

packages/web-components/src/components/masthead/cloud/cloud-masthead-composite.ts Outdated Show resolved Hide resolved

annawen1 reviewed Mar 24, 2021

View reviewed changes

packages/web-components/src/components/masthead/cloud/cloud-masthead-composite.ts Outdated Show resolved Hide resolved

proeung reviewed Mar 24, 2021

View reviewed changes

proeung suggested changes Mar 24, 2021

View reviewed changes

asudoh reviewed Mar 25, 2021

View reviewed changes

packages/web-components/src/components/masthead/cloud/cloud-masthead-profile.ts Outdated Show resolved Hide resolved

huntermacd added 2 commits March 26, 2021 11:10

feat(cloud-masthead): wire-up menu items, remove cruft, extend profile

66c32ff

feat(cloud-masthead): wip wrapping in feature flag

70085a3

huntermacd added 2 commits March 31, 2021 12:05

feat(cloud-masthead): create separate story, move/wire-up cookie logic

bf6e668

feat(cloud-masthead): add tests for cloud profile

1dfcb6d

huntermacd marked this pull request as ready for review April 1, 2021 13:15

huntermacd requested review from asudoh and annawen1 April 1, 2021 13:15

annawen1 added feature flag package: web components Work necessary for the IBM.com Library web components package labels Apr 1, 2021

proeung approved these changes Apr 8, 2021

View reviewed changes

huntermacd added 3 commits April 9, 2021 12:05

fix(cloud-masthead): update tests

3973a9a

feat(cloud-masthead): add updated snapshots

45cf5d6

Merge branch 'master' into cloud-masthead-variant

d9bdcf0

huntermacd force-pushed the cloud-masthead-variant branch from 494a7ed to d9bdcf0 Compare April 9, 2021 16:45

jeffchew requested changes Apr 9, 2021

View reviewed changes

shixiedesign added the Needs design approval PRs on feature requests and new components have to get design approval before merge. label Apr 12, 2021

huntermacd and others added 2 commits April 12, 2021 09:57

fix(cloud-masthead): remove duplicate js-cookie dependency

33cf590

Merge branch 'master' into cloud-masthead-variant

8909041

fix(cloud-masthead): set up login cookie status util

864d893

Merge branch 'master' into cloud-masthead-variant

7c2066c

annawen1 mentioned this pull request Apr 12, 2021

feat(cloud-megamenu): megamenu tabbing functionality foundation #5751

Merged

annawen1 approved these changes Apr 12, 2021

View reviewed changes

jeffchew approved these changes Apr 12, 2021

View reviewed changes

annawen1 added Ready to merge Label for the pull requests that are ready to merge and removed Needs design approval PRs on feature requests and new components have to get design approval before merge. labels Apr 12, 2021

kodiakhq bot merged commit ae52b30 into carbon-design-system:master Apr 12, 2021

jeffchew mentioned this pull request Apr 13, 2021

Carbon for ibm.com April 2021 Release Plan (v1.18.0) #5015

Closed

huntermacd deleted the cloud-masthead-variant branch April 15, 2021 15:20

proeung mentioned this pull request Apr 15, 2021

fix(masthead): global styles updates #5716

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cloud-masthead): add Cloud-specific container and composite components #5593

feat(cloud-masthead): add Cloud-specific container and composite components #5593

huntermacd commented Mar 24, 2021 •

edited

Loading

ibmdotcom-bot commented Mar 24, 2021 •

edited

Loading

ibmdotcom-bot commented Mar 24, 2021 •

edited

Loading

ibmdotcom-bot commented Mar 24, 2021 •

edited

Loading

annawen1 Mar 24, 2021 •

edited

Loading

proeung Mar 24, 2021

proeung Mar 24, 2021

annawen1 Mar 24, 2021

proeung Mar 25, 2021

proeung Mar 24, 2021

annawen1 Mar 25, 2021 •

edited

Loading

proeung Mar 25, 2021

annawen1 Mar 26, 2021

proeung Mar 26, 2021

annawen1 Mar 29, 2021

proeung Mar 30, 2021

proeung left a comment

asudoh left a comment •

edited

Loading

huntermacd commented Mar 30, 2021 •

edited

Loading

annawen1 commented Mar 30, 2021

asudoh commented Mar 31, 2021 •

edited

Loading

annawen1 commented Mar 31, 2021

ibmdotcom-bot commented Apr 2, 2021 •

edited

Loading

proeung left a comment

annawen1 commented Apr 8, 2021

proeung commented Apr 9, 2021

annawen1 commented Apr 9, 2021

jeffchew commented Apr 9, 2021

proeung commented Apr 9, 2021

jeffchew left a comment

proeung commented Apr 12, 2021 •

edited

Loading

huntermacd commented Apr 12, 2021 •

edited

Loading

annawen1 commented Apr 12, 2021

proeung commented Apr 12, 2021 •

edited

Loading

annawen1 left a comment

feat(cloud-masthead): add Cloud-specific container and composite components #5593

feat(cloud-masthead): add Cloud-specific container and composite components #5593

Conversation

huntermacd commented Mar 24, 2021 • edited Loading

Related Ticket(s)

Description

Changelog

ibmdotcom-bot commented Mar 24, 2021 • edited Loading

ibmdotcom-bot commented Mar 24, 2021 • edited Loading

ibmdotcom-bot commented Mar 24, 2021 • edited Loading

annawen1 Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annawen1 Mar 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proeung left a comment

Choose a reason for hiding this comment

asudoh left a comment • edited Loading

Choose a reason for hiding this comment

huntermacd commented Mar 30, 2021 • edited Loading

annawen1 commented Mar 30, 2021

asudoh commented Mar 31, 2021 • edited Loading

annawen1 commented Mar 31, 2021

ibmdotcom-bot commented Apr 2, 2021 • edited Loading

proeung left a comment

Choose a reason for hiding this comment

annawen1 commented Apr 8, 2021

proeung commented Apr 9, 2021

annawen1 commented Apr 9, 2021

jeffchew commented Apr 9, 2021

proeung commented Apr 9, 2021

jeffchew left a comment

Choose a reason for hiding this comment

proeung commented Apr 12, 2021 • edited Loading

huntermacd commented Apr 12, 2021 • edited Loading

annawen1 commented Apr 12, 2021

proeung commented Apr 12, 2021 • edited Loading

annawen1 left a comment

Choose a reason for hiding this comment

huntermacd commented Mar 24, 2021 •

edited

Loading

ibmdotcom-bot commented Mar 24, 2021 •

edited

Loading

ibmdotcom-bot commented Mar 24, 2021 •

edited

Loading

ibmdotcom-bot commented Mar 24, 2021 •

edited

Loading

annawen1 Mar 24, 2021 •

edited

Loading

annawen1 Mar 25, 2021 •

edited

Loading

asudoh left a comment •

edited

Loading

huntermacd commented Mar 30, 2021 •

edited

Loading

asudoh commented Mar 31, 2021 •

edited

Loading

ibmdotcom-bot commented Apr 2, 2021 •

edited

Loading

proeung commented Apr 12, 2021 •

edited

Loading

huntermacd commented Apr 12, 2021 •

edited

Loading

proeung commented Apr 12, 2021 •

edited

Loading