fix(sticky-header): do not consider vertical ToC height in calculations #10605
Conversation
jkaeser
added
package: utilities
|
Deploy preview created for package Built with commit: eebbfc29f2bea5aa3ee599e625ceb40efaea1973 |
|
Deploy preview created for package Built with commit: eebbfc29f2bea5aa3ee599e625ceb40efaea1973 |
|
Deploy preview created for package Built with commit: eebbfc29f2bea5aa3ee599e625ceb40efaea1973 |
|
Deploy preview created for package Built with commit: eebbfc29f2bea5aa3ee599e625ceb40efaea1973 |
jkaeser
requested review from
kennylam,
ariellalgilmore,
m4olivei,
pjudge and
andy-blum
| @@ -114,7 +114,7 @@ class StickyHeader { | |||
| const leadspaceSearchBar = component.shadowRoot.querySelector( | |||
| `.${prefix}--search-container` | |||
| ); | |||
| this._leadspaceWithSearchBar = leadspaceSearchBar; | |||
| this._leadspaceSearchBar = leadspaceSearchBar; | |||
There was a problem hiding this comment.
This and the following similar change was done to match the name of the property defined in this class's constructor.
|
Deploy preview created for package Built with commit: eebbfc29f2bea5aa3ee599e625ceb40efaea1973 |
|
We should probably update the storybook story that helps demo this utility. Can we add a knob to add the L1 menu? |
|
Deploy preview created for package Built with commit: eebbfc29f2bea5aa3ee599e625ceb40efaea1973 |
@andy-blum Looks like we have a knob for that already. 
|
|
Deploy preview created for package Built with commit: eebbfc29f2bea5aa3ee599e625ceb40efaea1973 |
andy-blum
added
the
Ready to merge
Related Ticket(s)
Description
The StickyHeader utility has logic for how to calculate the total height of elements on a given web page that are "stuck" to the top of the viewport. It exposes a CSS custom property,
--dds-sticky-header-height, with this calculated value for other page elements to use.In AEM, we were seeing this value be set much higher than the actual sum height of all stuck elements. It turns out we were adding the height of
<dds-table-of-contents>elements that are configured to use the vertical presentation, which is unnecessary.Changelog
Changed
<dds-table-of-contents>in the vertical orientation from--dds-sticky-header-heightcalculation when appropriate.<dds-table-of-contents>in the horizontal orientation from--dds-sticky-header-heightcalculation until it is sticking to the top of the viewport.<dds-leadspace-with-search>, allowing those elements to be properly factored into calculations.Testing Instructions
Visit the Dotcom Shell deploy preview's various Stories and verify the
--dds-sticky-header-heightcustom property set on the root element has a value that matches the current "stuck" elements' height.