Skip to content

Conversation

@castastrophe
Copy link
Contributor

@castastrophe castastrophe commented Aug 25, 2020

pfe-navigation

Styles

  • line height bug on menu items (#1454)
  • add min-width to mobile menu icon (#781)
  • autoprefixer warnings (#1214)
  • position of caret on mobile menu item with no tray (#795)
  • update styles to use unprefixed attributes (#1501)
  • ability to remove heading tags in pfe-navigation without visual changes (#773)

JS

  • doesn't work with Angular (#919)
  • doesn't render slot="mobile-login" unless slot"mobile-language" exists (#1106)
  • links inside pfe-modal inside pfe-navigation don't work (#873)
  • mobile links need event hooks (#945)

Possible candidate

  • should have a general purpose, utility slot for secondary links (#759)

Preview

Link(s) to demo page(s) where this element can be viewed:

Testing instructions

  • Navigation class: pfe-navigation__main--menu-label should have the same line-height as pfe-navigation-item [slot=trigger]
  • Mobile menu item should be at least 45px wide
  • Caret on the mobile navigation accordion should be vertically centered in the space
  • Navigation component should upgrade with no errors in angular
  • Should render a mobile-login slot even if no mobile-language slot is provided
  • If a modal is used inside the navigation component, links inside the modal should work as expected
  • Using p tags in the navigation element should not result in visual changes
  • On click, mobile links should surface events
  • There should be no autoprefixer warnings in the console during sass compilation

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 68 (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

Check off items as they are completed. Feel free to delete items if they are not applicable.

  • 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.
  • Browser testing passed.
  • Repository compiles and tests pass.
  • Changelog updated (not needed for documentation updates).
  • 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!

@castastrophe castastrophe added demo Updating demo pages functionality Functionality, typically pertaining to the JavaScript. styles An issue or PR pertaining only to CSS/Sass labels Aug 25, 2020
@castastrophe castastrophe added run e2e Trigger automated visual regression tests and removed run e2e Trigger automated visual regression tests labels Sep 1, 2020
@castastrophe castastrophe added the tests Related to testing label Sep 4, 2020
@castastrophe castastrophe added tools Development and build tools work in progress POC / Not ready for review labels Sep 11, 2020
@bennypowers bennypowers deleted the fix-issue-873-language-modal branch June 25, 2024 07:38
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 functionality Functionality, typically pertaining to the JavaScript. priority: high Severity level: 1 styles An issue or PR pertaining only to CSS/Sass tests Related to testing work in progress POC / Not ready for review

Projects

None yet

2 participants