Skip to content

Conversation

@LyndseyR
Copy link
Contributor

Title of this pull request

  • Link to discovery documentation (should contain all the planning and requirements for this work):
    • See the DISCOVERY.md template at the root of the project
  • Link(s) to demo pages where this element can be viewed:

What has changed and why

Summarize files edited as part of this MR along with a brief description of what was changed/why.

  • pfe-navigation/demo/index.html - making this match the mockups to be shared out
  • pfe-navigation/src/pfe-navigation--lightdom-2.scss - making temporary file that will merge over into --lightdom.scss when pfe-navigation branch is merged in to master.

Testing instructions

Be sure to include detailed instructions on how your update can be tested by another developer.

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 60.7.2 (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
  • Galaxy S9 Firefox
  • iPhone X Safari
  • iPad Pro Safari
  • Pixel 3 Chrome

Your repository infrastructure updates should work for at least:

  • Node v8.x
  • NPM v7.x

Ready-for-merge Checklist

  • Expected files: all files in this pull request are related to one feature request or issue (no stragglers)?
  • Did automated tests pass?
  • Did browser testing pass?
  • Did you update or add any necessary documentation (README.md, WHY.md, etc.)?
  • Was this feature demo'd and the design review approved?
  • Did it get a LGTM after the last commit?
  • Did you update the CHANGELOG.md file with a summary of this update?

Be sure to share your updates with the patternfly-elements-contribute@redhat.com mailing list!

@starryeyez024 starryeyez024 changed the base branch from master to pfe-navigation September 5, 2019 14:37
}

.border-top {
/* .border-top {
Copy link
Contributor

Choose a reason for hiding this comment

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

@LyndseyR you can delete this completely from the demo file since this is being handled separately

<h2 slot="logo" style="padding: 0 0 8px 0;line-height: 1;">
<a href="https://redhat.com">
<img class="redhat-logo" src="https://via.placeholder.com/150x50.png" class="screen" title="Red Hat"/>
<img src="/profiles/rh/themes/redhatdotcom/img/logo.svg" class="screen" title="Red Hat" style="width: 135px; height: 100%;">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you revert this to a more generic image? We don't want the red hat logo on the generic open source project's navigation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Roger!

<div class="pfe-nav-grid">
<div class="column">
<div class="pfe-link-list">
<p class="pfe-link-list--header">Platforms</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of p tags, can you update these headers to the next h-level down from the navigation item's h-tag?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also have it link up the header to the list of links using aria-labelledby when it upgrades? These IDs can be autogenerated if they don't exist in the lightDOM. I did something similar in the pfe-footer branch/element.

For example:

<h3 id="pfe-nav__h-cfgeh2343nsah373ja">Platforms</h3>
<ul aria-labelledby="pfe-nav__h-cfgeh2343nsah373ja">
...
</ul>

Copy link
Contributor

Choose a reason for hiding this comment

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

These are lightDOM-only styles that Lyndsey is working on so the web component won't be able to handle the addition of the aria-labelledby tag but we can manually add it to the markup and share that with the dev team (Drupal will be outputting this markup).

Copy link
Contributor

@markcaron markcaron Sep 13, 2019

Choose a reason for hiding this comment

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

Ok, right. Missed the "lightDOM" part of the criteria for this PR. Sounds good! When this becomes its fully-fledged web component, we'll handle it in a similar way (JS upgrade).

Copy link
Contributor

Choose a reason for hiding this comment

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

@LyndseyR Let's do this via the analytics branch that's coming next sprint - don't worry about incorporating these aria labels yet.

Lyndsey Mitchel added 2 commits September 13, 2019 21:09
@castastrophe
Copy link
Contributor

👍 Thanks for this Lyndsey! It looks great. We'll resolve the testing errors in pfe-navigation branch.

@castastrophe castastrophe merged commit c24ad44 into pfe-navigation Sep 13, 2019
kylebuch8 pushed a commit that referenced this pull request Oct 3, 2019
* initial pfe-navigation changes

* ch ch ch changes

* initial pfe-navigation commit

* Bring back the pfe-link-list component; updates to structure of navigation component

* Revert "initial pfe-navigation commit"

This reverts commit 0148e8b.

* Getting navigation to a workabout/style-able place

* Working on theming the navigation-items based on latest mock-ups

* Make the navigation item with icons smaller text

* Got icons working again; polishing states for navigation-items; cleaning up styles for mobile nav

* Apply icons using variables

* Adding styles for the mobile login and language switchers

* Update navigation styles

* Add sticky navigation setting and set up click outside to close

* Updating cloning

* Add overflow settings to tray region

* Tweaks to overflow settings inside the navigation-item tray

* Port the mobile login and language to shadowDOM

* Updating variables to map more closely to the design system

* Adjusting where JS is defined for Safari

* Trying to get Safari to render correctly

* Wrap mobile search in a container so it is easier to hide-show on mobile

* Set up the resize to hide the menu when over 996px

* Add path support for Safari and FF

* Add back navigation-main for markup

* Add a few more styles to the link-list component to match mock-ups

* Replace Red Hat logo with generic placeholder

* Remove unused this.slot declaration which was causing a slot attribute to be added to the wrapper

* Updating styles via code review feedback with Kendall

* Updates re: design feedback + fix bug with close item state

* Fix the focus-within issue; remove selector, style on parent trigger element

* Suppress the fallback link when upgraded

* Shoring up support for direct links in the top-navigation

* Add tests tot he demo page for dynamic content injectin

* Working on fallback and improved styles for edge

* Working on edge styles + add more information to the custom toggled event to make content switching easier

* Edge is looking good; fallback styles

* Edge style fixes

* Adding more comments; wrapping elements in a warning if no light DOM

* Add a closeAllNavigationItems function to more consistently handle the close items event in the main navigation element

* Add a more dynamic way to render mobile-search content based on what's provided in the search slot using an attribute: pfe-navigation--mobile-search

* Remove comments and fix background color styles

* moving script tag inside closing body tag

Also fixed backticks that IE11 doesn't care for.

* moved the creation of PfeNavigation to top of the list

I'm not really sure why this fixes things but order seems to matter here. I found the issue by getting rid of PfeNavigationItem and noticed that when I brought it back in, it created an error with the polyfill. Moving PfeNavigation ahead of it in the creation process seems to fix the issue.

* Reworking accordion to work inside of the navigation component

* Updating accordion styles to support hooks and additional properties

* Revert accordion updates; moved to new branch: accordion-enhancements-updates

* Update navigation to use bubble up events on click

* Standardize on open and close custom events

* Pull out link-list component; fix gulpfile configfactory variable; add back accordion mixins

* Update examples and test files to remove link-list

* Update height variable on sticky state for more graceful transition between sticky and non-sticky state

* Code review feedback; remove console output

* Update markup and imports for storybook instance

* Tidy up navigation storybook, update markup with notes

* Remove demo css for now

* Working with the storybook settings; cleaning up the demo file; adjusting the padding on the nav styles for mobile; add navigation-item schema

* Add light dom styles for tray region; use layouts instead of custom styles

* Tweaking implementation of mobile nav for better IE11 experience

* Add a direct link handler since link is disabled on upgrade

* Fix overlay showing up when escape key is hit twice

* Adding framework for light dom styles for link list

* Fix the logic around which elements fire a close on screen resize

* Push up updates to the styles based on code review feedback; updated pfe-radio to support fallback values

* To prevent changes to existing styles, adjusting the radio function to check for a global variable, $USE-FALLBACK which defaults to false

* Update schemas, add documentation to README

* Push up fix for arrow on to show up on hover; pfe-icon prefixes added to stylesheet; pfe-sticky prefix in JS fixed

* Update versions listed in the package.json for navigation

* Add the pfe prefix to the icon in navigation stylesheet; updated package versions; add cursor styles to navigation item trigger element

* Building the mobile navigation as light DOM; in a broken state but closer to finished now

* pfe-navigation: Cleaning up desktop v. mobile view

* Styles for the updated mobile navigation working in all screen sizes

* Adjusting styles for the nested items, better chevron visibility

* Updating the breakpoint approach, using attributes to toggle visibility

* Resurrect mobile search fields

* Letting breakpoints flow from the JS

* Need to remove the consoles but working through open and close states

* Add exception for hidden attribute on the menu item

* Cleaning up trigger states in toggleHandler

* Click events are all working at this point

* Click events and keyboard navigation working

* Add attributes so styles can be more efficient and clear

* Add close icon

* Ensure escape exits open items even if not in the active state list

* Add todo note

* removing bluebird dependency

* Adjust events to generic toggle; add keymappings for older browsers

* Move navItem collection for pfe-navigation-main to init function

* Adjusting layout for mobile menu for better collapsibility

* Update package to latest release; clean up event firing on nav

* Refactor styles for better fallbacks, using slots not the shadow classes

* Live code review feedback coding updates

* Updating packages; debugging Edge errors

* Update JS to support Edge upgrade; separate navigation files (add compilation task support in gulpfile)

* Adjusting styles for edge compatibility

* Navigation is working in IE11 - note there are debug messages in this commit

* Use pfe-icon to fix IE11 upgrade bug

* Adjusting styles for IE11 compatibility

* Update package for navigation version

* IE11 is upgrading!

* Getting main navigation a little closer in IE11

* Pfe navigation light dom (#484)

* pfe-navigation-light-dom added lightdom styles for desktop, still need to add mobile styles in shadowdom

* pfe-navigation-light-dom added styles for desktop, still working through tablet

* origin/pfe-navigation-light-dom added backup index.html file and updated styles to adjust branch testing bugs

* pfe-navigation-light-dom added styles that addressed the rest of round 1 branch testing

* pfe-navigation-light-dom updated styles for all sections to call in grid and be wrapped in generic column

* pfe-navigation-light-dom added fallbacks for IE

* pfe-navigation-light-dom cleaned up footer breakpoints so they're aligned when in columns

* Merge code with pfe-nav base

* pfe-navigation-light-dom updated class names to have prefix, added auto-flow grid for footer md breakpoint, added 3 columns for tray styles on md breakpoint

* pfe-navigation-light-dom updated class naming to follow BEM syntax

* Updating branch based on code review feedback

* Set pfe-icon as a dependency of pfe-navigation-item

* Add support for temp compilation with files defined in package.json

* Style tweaks and adjustments based on branch testing; setting pfe-nav to private

* Fix gulp build so that all assets are added to the temp directory and if no files array exists in the package.json, all those files are copied to the root

* Remove themes from spandx config; remove tab focus trap from navigation

* Update markup in the demo page, navigation readme

* Fix storybook rendering by adjusting outside listener to allow nesting inside another web component

* Clean up story; commenting out lightdom to debug styles

* Improving on the edge experience

* Tidying up edge experience

* Simplify edge, looking good!

* updating dynamically injected tray text

* Remove console.log, remove outline on link

* Add events to documentation

* Stub out tests

* Remove debug from nav-item; update example in test file; remove __last from template file

* adding tests for pfe-navigation-main

* test: adding more tests to pfe-navigation and pfe-navigation-item

* [dev]: Updates to icon to support fallback text, no fallback text, and collapsing on fail

* [dev]: Add TODO comment for icon hiding

* fix: using the cdn for the polyfills

We need this so it works with Netlify
diwanshi pushed a commit that referenced this pull request Oct 4, 2019
* initial pfe-navigation changes

* ch ch ch changes

* initial pfe-navigation commit

* Bring back the pfe-link-list component; updates to structure of navigation component

* Revert "initial pfe-navigation commit"

This reverts commit 0148e8b.

* Getting navigation to a workabout/style-able place

* Working on theming the navigation-items based on latest mock-ups

* Make the navigation item with icons smaller text

* Got icons working again; polishing states for navigation-items; cleaning up styles for mobile nav

* Apply icons using variables

* Adding styles for the mobile login and language switchers

* Update navigation styles

* Add sticky navigation setting and set up click outside to close

* Updating cloning

* Add overflow settings to tray region

* Tweaks to overflow settings inside the navigation-item tray

* Port the mobile login and language to shadowDOM

* Updating variables to map more closely to the design system

* Adjusting where JS is defined for Safari

* Trying to get Safari to render correctly

* Wrap mobile search in a container so it is easier to hide-show on mobile

* Set up the resize to hide the menu when over 996px

* Add path support for Safari and FF

* Add back navigation-main for markup

* Add a few more styles to the link-list component to match mock-ups

* Replace Red Hat logo with generic placeholder

* Remove unused this.slot declaration which was causing a slot attribute to be added to the wrapper

* Updating styles via code review feedback with Kendall

* Updates re: design feedback + fix bug with close item state

* Fix the focus-within issue; remove selector, style on parent trigger element

* Suppress the fallback link when upgraded

* Shoring up support for direct links in the top-navigation

* Add tests tot he demo page for dynamic content injectin

* Working on fallback and improved styles for edge

* Working on edge styles + add more information to the custom toggled event to make content switching easier

* Edge is looking good; fallback styles

* Edge style fixes

* Adding more comments; wrapping elements in a warning if no light DOM

* Add a closeAllNavigationItems function to more consistently handle the close items event in the main navigation element

* Add a more dynamic way to render mobile-search content based on what's provided in the search slot using an attribute: pfe-navigation--mobile-search

* Remove comments and fix background color styles

* moving script tag inside closing body tag

Also fixed backticks that IE11 doesn't care for.

* moved the creation of PfeNavigation to top of the list

I'm not really sure why this fixes things but order seems to matter here. I found the issue by getting rid of PfeNavigationItem and noticed that when I brought it back in, it created an error with the polyfill. Moving PfeNavigation ahead of it in the creation process seems to fix the issue.

* Reworking accordion to work inside of the navigation component

* Updating accordion styles to support hooks and additional properties

* Revert accordion updates; moved to new branch: accordion-enhancements-updates

* Update navigation to use bubble up events on click

* Standardize on open and close custom events

* Pull out link-list component; fix gulpfile configfactory variable; add back accordion mixins

* Update examples and test files to remove link-list

* Update height variable on sticky state for more graceful transition between sticky and non-sticky state

* Code review feedback; remove console output

* Update markup and imports for storybook instance

* Tidy up navigation storybook, update markup with notes

* Remove demo css for now

* Working with the storybook settings; cleaning up the demo file; adjusting the padding on the nav styles for mobile; add navigation-item schema

* Add light dom styles for tray region; use layouts instead of custom styles

* Tweaking implementation of mobile nav for better IE11 experience

* Add a direct link handler since link is disabled on upgrade

* Fix overlay showing up when escape key is hit twice

* Adding framework for light dom styles for link list

* Fix the logic around which elements fire a close on screen resize

* Push up updates to the styles based on code review feedback; updated pfe-radio to support fallback values

* To prevent changes to existing styles, adjusting the radio function to check for a global variable, $USE-FALLBACK which defaults to false

* Update schemas, add documentation to README

* Push up fix for arrow on to show up on hover; pfe-icon prefixes added to stylesheet; pfe-sticky prefix in JS fixed

* Update versions listed in the package.json for navigation

* Add the pfe prefix to the icon in navigation stylesheet; updated package versions; add cursor styles to navigation item trigger element

* Building the mobile navigation as light DOM; in a broken state but closer to finished now

* pfe-navigation: Cleaning up desktop v. mobile view

* Styles for the updated mobile navigation working in all screen sizes

* Adjusting styles for the nested items, better chevron visibility

* Updating the breakpoint approach, using attributes to toggle visibility

* Resurrect mobile search fields

* Letting breakpoints flow from the JS

* Need to remove the consoles but working through open and close states

* Add exception for hidden attribute on the menu item

* Cleaning up trigger states in toggleHandler

* Click events are all working at this point

* Click events and keyboard navigation working

* Add attributes so styles can be more efficient and clear

* Add close icon

* Ensure escape exits open items even if not in the active state list

* Add todo note

* removing bluebird dependency

* Adjust events to generic toggle; add keymappings for older browsers

* Move navItem collection for pfe-navigation-main to init function

* Adjusting layout for mobile menu for better collapsibility

* Update package to latest release; clean up event firing on nav

* Refactor styles for better fallbacks, using slots not the shadow classes

* Live code review feedback coding updates

* Updating packages; debugging Edge errors

* Update JS to support Edge upgrade; separate navigation files (add compilation task support in gulpfile)

* Adjusting styles for edge compatibility

* Navigation is working in IE11 - note there are debug messages in this commit

* Use pfe-icon to fix IE11 upgrade bug

* Adjusting styles for IE11 compatibility

* Update package for navigation version

* IE11 is upgrading!

* Getting main navigation a little closer in IE11

* Pfe navigation light dom (#484)

* pfe-navigation-light-dom added lightdom styles for desktop, still need to add mobile styles in shadowdom

* pfe-navigation-light-dom added styles for desktop, still working through tablet

* origin/pfe-navigation-light-dom added backup index.html file and updated styles to adjust branch testing bugs

* pfe-navigation-light-dom added styles that addressed the rest of round 1 branch testing

* pfe-navigation-light-dom updated styles for all sections to call in grid and be wrapped in generic column

* pfe-navigation-light-dom added fallbacks for IE

* pfe-navigation-light-dom cleaned up footer breakpoints so they're aligned when in columns

* Merge code with pfe-nav base

* pfe-navigation-light-dom updated class names to have prefix, added auto-flow grid for footer md breakpoint, added 3 columns for tray styles on md breakpoint

* pfe-navigation-light-dom updated class naming to follow BEM syntax

* Updating branch based on code review feedback

* Set pfe-icon as a dependency of pfe-navigation-item

* Add support for temp compilation with files defined in package.json

* Style tweaks and adjustments based on branch testing; setting pfe-nav to private

* Fix gulp build so that all assets are added to the temp directory and if no files array exists in the package.json, all those files are copied to the root

* Remove themes from spandx config; remove tab focus trap from navigation

* Update markup in the demo page, navigation readme

* Fix storybook rendering by adjusting outside listener to allow nesting inside another web component

* Clean up story; commenting out lightdom to debug styles

* Improving on the edge experience

* Tidying up edge experience

* Simplify edge, looking good!

* updating dynamically injected tray text

* Remove console.log, remove outline on link

* Add events to documentation

* Stub out tests

* Remove debug from nav-item; update example in test file; remove __last from template file

* adding tests for pfe-navigation-main

* test: adding more tests to pfe-navigation and pfe-navigation-item

* [dev]: Updates to icon to support fallback text, no fallback text, and collapsing on fail

* [dev]: Add TODO comment for icon hiding

* fix: using the cdn for the polyfills

We need this so it works with Netlify
@castastrophe castastrophe deleted the pfe-navigation-light-dom branch October 15, 2019 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants