-
Notifications
You must be signed in to change notification settings - Fork 158
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
[Dotcom-shell]: remove scrolling logic and use new utility #8472
Comments
One thing we need to consider prior to yanking out the dotcom shell's scrolling logic is how do we want to handle the leadspace-with-search component? That's not currently accounted for in the new scrolling utility. How should it interact with the masthead/toc to stick to the top of the page? |
Hi @andy-blum! sorry didn't see your comment before. There are specs on the interaction in here: https://ibm.ent.box.com/file/814160426264?s=r2wmgutqhybbmey96zv8rvjjufsw6ib4. @oliviaflory lmk if there is anything else! |
@andy-blum Can you note the issue you're having with getting the scrolling utility and making it work with the Leadspace-with search component? |
Right now, we're trying to handle everything on scroll, through a throttled event listener. The throttler worked enough before we tried to add in the leadspace-with-search component, but as we add in more logical complexity to the handler, we're blocking the main thread of the JS and causing some jitter in the page. Every single time the scroll handler runs, it's got to go through the entire calculation of determining which components are on the page, if they should stick, and then altering their With regards to the leadspace-with-search component in particular, the main issue I encountered was that the search bar does not cleanly separate from the document flow. When it moves to a fixed position, I was unable to get it to stick to the header, adhere to the carbon grid, and not cause the document to reflow when I set it to |
@andy-blum Thanks for sharing. Do you have a branch where we could try to see the problem locally to get a better understanding of the issue? |
@ariellalgilmore I had deleted my local branch, but remembered the majority of the work I did to get started. I've made a new local branch and it's about as close as I've gotten so far. I'll publish it to my fork and share the link with you here. |
…om-shell code (#8684) ### Related Ticket(s) Closes #8486 #8472 ### Description This PR is a followup top #8434 in which the dotcom-shell's scroll handling was mimicked in a utility class. This PR aims to cleanup the storybook integration and removes now unnecessary code from the dotcom-shell-composite class. ### Changelog **Changed** - Updated storybook story for new dotcom-shell scrolling utility **Removed** - Scroll handling logic removed from the dotcom-shell component
Engineering info:
Description
Follup up work on #8240.
#8240 created a new scrolling utility, but did not yet remove scrolling logic from
dds-dotcom-shell
. We need to remove the logic fromdds-dotcom-shell
as well as test functionality withLeadspaceWithSearch
in the new storyDotcom-shell -> withoutShell
Component(s) impacted
Dotcom-shell
masthead (LO, L1)
TOC (vertical, horizontal)
Leadspace with search
Universal Banner
Browser
No response
Carbon for IBM.com version
Canary
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
carbon for ibm.com
Package
@carbon/ibmdotcom-web-components
CodeSandbox example
N/A
Steps to reproduce the issue (if applicable)
No response
Release date (if applicable)
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: