Skip to content

Conversation

@castastrophe
Copy link
Contributor

@castastrophe castastrophe commented May 27, 2021

Related issues

What has changed and why

  • Add support to pfe-accordion-header for slotted content to be read in and rendered (this allows us to use pfe-accordion inside another template and pass a slot into the pfe-accordion-header)
  • Add PfeAccordion to pfe-jump-links-nav as a dependency
  • Add support for the following events:
    • pfe-jump-links-panel:active-navItem: when a new item in the nav becomes active, this event is fired for analytics; this is no longer handled by the panel but the event name persists for backwards compatibility
    • pfe-jump-links-panel:change: fired when the panel or section elements change; used by the autobuilder; calls _panelChangedHandler
    • pfe-jump-links-nav:stuck
    • resize: on page resize, the rebuild method is called
    • scroll: handles updating the active states when the page is scrolled
    • keyup: this is a stub and needs to be addressed at a later date (@todo open issue for this)
  • Added the following attributes to pfe-jump-links-nav:
    • offset: the manually set distance from the top of the viewport to the top of the navigation element; this is an override and does not account for pfe-navigation when used.
    • mobile-breakpoint: max-width for when to switch to the pfe-accordion for mobile rendering
    • accordion-collapse-timing: how long to wait after clicking a child element before collapsing the open accordion
    • stuck: represents the stickiness state
    • no-header: boolean attribute to allow you to opt-out of rendering the header section (header becomes screen-reader only)
  • Added the following attributes to pfe-jump-links-panel:
    • spacers: opt-in attribute for injecting the spacer elements; not recommended but allows backward compatibility

Going forward, the only required component to add to the page is pfe-jump-links-nav. Panel is an optional wrapper to make targeting more efficient. All functionality is being handled inside the nav element. Panel and sections can be manually defined via JavaScript API call to the pfe-jump-links-nav component.

Test cases

  • Initialization
    • URL
    • Validate that on page load, the standard heading is selected in the top horizontal navigation and the Section 1 region is selected in the first visible vertical jump links.
  • Stickiness
    • URL
    • On scroll, the horizontal and vertical elements should stick to the top of the viewport while it's content is visible; it should account for the height of pfe-navigation if it exists and any horizontal sticky elements
    • Edge case: If 2 horizontal sticky elements are present, the one later in the DOM will overlap the other one.
  • Active state
    • URL
    • On scroll, the active state in the navigation should update when the section scrolls close to the top of the viewport.
  • Click event
    • URL
    • On clicking a navigation item on either horizontal or vertical nav, the page should smooth scroll to that section so that the top of the section is visible close to the top of the viewport (accounting for sticky elements) with at least a 20px breathing space (unless offset is manually set in the mark-up)
    • No in-between sections should show as active while the scroll is happening
    • Scroll controlled active states should resume after the scrolling event is complete

Browser requirements

Your component should work in all of the following environments:

  • Latest 2 versions of Edge
  • Internet Explorer 11 (should be useable, not pixel perfect)
  • Latest 2 versions of Firefox (one on Mac OS, one of Windows OS)
  • Firefox 78 (or latest version for Red Hat Enterprise Linux distribution)
  • Latest 2 versions of Chrome (one on Mac OS, one of Windows OS)
  • Latest 2 versions of Safari
  • Android mobile device (such as the Galaxy S9)
  • Apple mobile device (such as the iPhone X)
  • Apple tablet device (such as the iPhone Pro)

Ready-for-merge Checklist

  • Expected files: all files in this pull request are related to one request or issue (no stragglers or scope-creep).
  • [ ] Tests have been updated to cover these changes. TODO return to this after code freeze
  • Browser testing passed.
  • Changelog updated.
  • Documentation (README.md, WHY.md, etc.) updated or added.
  • Link to the demo recording:
  • Approved by designer.

Merging

Please squash when merging and ensure your commit message uses conventional commit formatting.

Be sure to share your updates with the [email protected] mailing list!

@github-actions github-actions bot added work in progress POC / Not ready for review demo Updating demo pages functionality Functionality, typically pertaining to the JavaScript. styles An issue or PR pertaining only to CSS/Sass labels Jun 3, 2021
@github-actions github-actions bot added the tools Development and build tools label Jun 4, 2021
@castastrophe castastrophe changed the title WIP: Fix jump links experience fix: Jump links styles and browser fixes Jun 4, 2021
@castastrophe castastrophe added feature New feature or request next release PRs that need to merge before the next release goes out priority: high Severity level: 1 ready: code review Ready for code review! ready: design review run e2e Trigger automated visual regression tests size: lg Sizing label; indicates a very difficult task or large amount of work and removed work in progress POC / Not ready for review labels Jul 8, 2021
@zhawkins
Copy link
Contributor

zhawkins commented Jul 8, 2021

branch testing looks good 👍

@castastrophe castastrophe changed the title fix: Jump links styles and browser fixes fix: Jump links styles and browser fixes Jul 8, 2021
@castastrophe castastrophe changed the title fix: Jump links styles and browser fixes fix: Jump links styles and browser fixes Jul 8, 2021
Copy link
Contributor

@heyMP heyMP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@castastrophe castastrophe merged commit af4bc70 into master Jul 8, 2021
@castastrophe castastrophe deleted the fix-jump-links-experience branch July 8, 2021 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

demo Updating demo pages docs Documentation updates feature New feature or request functionality Functionality, typically pertaining to the JavaScript. next release PRs that need to merge before the next release goes out priority: high Severity level: 1 ready: code review Ready for code review! ready: design review run e2e Trigger automated visual regression tests size: lg Sizing label; indicates a very difficult task or large amount of work styles An issue or PR pertaining only to CSS/Sass tests Related to testing tools Development and build tools

Projects

None yet

4 participants