carbon-design-system / carbon-for-ibm-dotcom

Carbon for IBM.com is based on the Carbon Design System for IBM
https://www.ibm.com/standards/carbon/
Apache License 2.0
268 stars 156 forks source link

[TOC - Horizontal]: stickyOffSet causes ToC to hide titles and creates gap above ToC in mobile/tablet #7643

Closed EAVilleda closed 2 years ago

EAVilleda commented 2 years ago

Engineering info:


Description

When an item is clicked on in the horizontal table of contents, the page should scroll to the related title. However, we have a site navigation that requires the table of contents to be offset by 48 or 96, depending on if first level navigation is also included. In either case, clicking on an item in the ToC causes the title of the content to be hidden behind the ToC. image

Another related issue is that the stickyOffSet is causing the ToC in mobile/tablet to be lower on the page. This causes a gap/space above the ToC because the site navigation bar is hidden. This gap is most noticeable when the user scrolls down; the page's content can be seen in this gap above the ToC.

image

Component(s) impacted

Table of Content Horizontal https://www.ibm.com/standards/carbon/web-components/?path=/docs/components-table-of-contents--horizontal

Browser

Chrome

Carbon for IBM.com version

v1.26.0

Severity

Severity 3 = The problem is visible or noticeable to users but does not impede the usability or functionality. Affects minor functionality, has a workaround.

Application/website

AEM (Marketing)

Package

@carbon/ibmdotcom-web-components

CodeSandbox example

https://codesandbox.io/s/competent-feather-omk49?file=/index.html

Steps to reproduce the issue (if applicable)

CodeSandbox example isn't working due to a build error. To reproduce in Storybook.

  1. Inspect the page https://www.ibm.com/standards/carbon/web-components/iframe.html?id=components-table-of-contents--horizontal
  2. Add stickyoffset="48" to dds-table-of-contents element
  3. Click on an item in the table of contents and observe where the page jumps to.
  4. Resize the window smaller to test how the gap persists in mobile/tablet view.

image

Release date (if applicable)

No response

Code of Conduct

EAVilleda commented 2 years ago

We also have a second issue related to stickyOffSet. Specifically that the stickyOffSet is causing the ToC in mobile/tablet to be lower on the page so there is a space above the ToC. Should I create a separate ticket for this issue or can it be addressed here as well? image

annawen1 commented 2 years ago

@EAVilleda I believe we can address it in the same ticket if it is same severity. One thing, make sure to add the details of the related issue in the description as well! Sometimes info in the comments are overlooked.

EAVilleda commented 2 years ago

@annawen1 I've updated the ticket with details regarding the mobile issue.

proeung commented 2 years ago

@RobertaJHahn @jeffchew This is the epic that I mentioned (https://github.com/carbon-design-system/carbon-for-ibm-dotcom/issues/6994). The reworking of the sticky behavior might address some of the offset/overlapping issues that we're seeing on the TOC and other components with the stick-to-top behavior. Nonetheless, this issue can be used to address a workaround solution until we figure out the best route for the EPIC above.

andy-blum commented 2 years ago

@EAVilleda while I was able to replicate your issue following the steps you outlined, I was not able to replicate the issue when the table of contents is used within the dotcom-shell component. I believe we have some logic in there to help avoid the issue you're running into. Can you try utilizing the dotcom-shell and see if that addresses your issue?

EAVilleda commented 2 years ago

@andy-blum We currently don't have the dotcom-shell in our project, and it can potentially impact our templates to try and include it. Can you explain the purpose of the dotcom-shell?

annawen1 commented 2 years ago

@EAVilleda The DotcomShell includes the <dds-masthead>, and <dds-footer> components, all wrapped in a UI shell using Carbon's grid. It also includes logic to control the interaction of the masthead with the table of contents and other components.

EAVilleda commented 2 years ago

@annawen1 I took a look at how the dotcom-shell is used in Storybook. It looks like it wraps around components between masthead and footer. But we won't be able to replicate this without changing the node structure of existing pages. Can we wrap the masthead and footer inside of the dotcom-shell tags as well, without side effects?

annawen1 commented 2 years ago

@EAVilleda probably not, since the dotcom-shell is a wrapper with the masthead and footer components, so you would be seeing double masthead and footer.

EAVilleda commented 2 years ago

@annawen1 Given that the component docs don't mention anything about dotcom-shell https://www.ibm.com/standards/carbon/web-components/?path=/story/components-masthead--default, wouldn't masthead be able to work standalone, without the shell?

annawen1 commented 2 years ago

@EAVilleda when we were working on the behavior of the masthead on scroll, we found that we needed to manipulate the style of other components (ie. Table of Contents, Leadspace With Search). We were also ran into issues with trying to make the masthead sticky when applying that logic on itself. All of this logic needed to be taken on by a parent component. So this logic was moved to the dotcom-shell. I'll speak with the team and see if there is anything we can do.

However, I am seeing the issues on this ticket even with the use of the dotcom-shell so this needs more investigation.

Are you able to provide a link to a dev environment that I can look at?

EAVilleda commented 2 years ago

@annawen1 This is a link to the page in Dev. https://dev-publish.np-roks.cms.cis.ibm.net/qa/solution-testing-ii-mr-jan-19-26 But we are not currently use dotcom-shell on the page.

proeung commented 2 years ago

@annawen1 I think one of the reasons why the <dds-domcom-shell> is not being utilized on the AEM templating side is because this bucket component doesn't render the Cloud Masthead and fetches Footer data from any other data endpoints that are defined (eg. Cloud, Storage, QRadar). I've created a ticket a while back to enable the Cloud masthead usage on dotcom-shell (see - https://github.com/carbon-design-system/carbon-for-ibm-dotcom/issues/7009).

Perhaps, we can explore ways that adopters like AEM can call for the different mastheads within the dotcom-shell component? If you have any guidance on how we can accomplish this via a sandbox example that would be great as this will help streamline how the dotcom-shell component can be utilized in AEM so that we can leverage the parent logic of the scroll to behavior that's already there.

annawen1 commented 2 years ago

@proeung currently different mastheads can't be utilized in the dotcom-shell so there would definitely need to be work done. I guess this makes #7009 a greater priority, so that AEM is able to utilize the dotcom-shell for all templates

proeung commented 2 years ago

@annawen1 I agree. I'll bring this up in my next issue-sharing review meeting with @jeffchew and @ljcarot. Thanks!

proeung commented 2 years ago

Closing out this issue in favor of this one (https://github.com/carbon-design-system/carbon-for-ibm-dotcom/issues/8240), which will address the TOC overlapping issue when the Dotcom-shell component is not being used.