Skip to content
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

Integrate Navigation and move Skip Link to Brand #5234

Merged
merged 228 commits into from
Feb 4, 2020

Conversation

tochwill
Copy link
Contributor

@tochwill tochwill commented Jan 22, 2020

Resolves #4571
Overall change:
This PR implements the following:

Integrate Psammead components and handle state for AMP and Canonical. (#4571 and #4570)
Add entry and exit animation for navigation dropdown, for AMP and Canonical. (#4579)
Move skip to content link from navigation to brand component (#2829)

Code changes:

  • Integrated the Psammead navigation components
  • Integrated newest version of Psammead brand component
  • Moved script link out of navigation into brand
  • Moved translations for navigation menu button into translations object
  • Added media query listener hook

  • I have assigned myself to this PR and the corresponding issues
  • I have added labels to this PR for the relevant pod(s) affected by these changes
  • I have assigned this PR to the Simorgh project

Testing:
The expected behaviour for this PR is as follows:

  • For all page types - the navigation remains as it currently behaves on live for screen widths > 599px.

  • For screen widths below 599px and below, for all page types:

    • The navigation links appear within a horizontally scrollable navigation, alongside a menu button.
    • When the menu button is selected, the navigation links in the scrollable navigation disappear, and a dropdown menu with the navigation links appears. The rest of the page is pushed down below this menu. This animates in on the canonical pages, but does not animate on AMP pages.
    • On selecting the menu button again, the dropdown menu disappears, and the scrollable menu reappears.
    • The skip to content link appears before the navigation in the header, and DOES NOT appear in the footer.
    • Known issue - the horizontally scrollable menu may not work until focus is within the menu on internet explorer. This was previously tested in the psammead component, and marked as won't fix (due to the likelyhood of a user browsing the site on IE at a screen width of below 600px, as well as levels of traffic for IE).
  • Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)

  • If necessary, I have run the local E2E non-smoke tests relevant to my changes (CYPRESS_APP_ENV=local CYPRESS_SMOKE=false npm run test:e2e:interactive)

  • This PR requires manual testing

AlistairGempf and others added 30 commits November 29, 2019 16:23
@PriyaKR
Copy link
Contributor

PriyaKR commented Jan 30, 2020

Blocked by ios testing.

@PriyaKR PriyaKR added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jan 30, 2020
@tochwill
Copy link
Contributor Author

tochwill commented Jan 31, 2020

Seeing a couple of issues on Opera Mini extreme mode (also seeing some live issues on Opera Mini) - having a chat about this today

@PriyaKR
Copy link
Contributor

PriyaKR commented Jan 31, 2020

@tochwill Have tested on ios using the heroku app url looks good but i am seeing the page content blocked in opera mini in extreme mode looks good in data savings high and off mode.

Also voice over on ios looks good.All good from testing perspective except opera mini issue.

@tochwill
Copy link
Contributor Author

tochwill commented Feb 3, 2020

@PriyaKR have spoken to Jim and Neil about Opera Mini issues, they'd like to go ahead with releasing and create new issues to fix Opera Mini issues this week.

@tochwill
Copy link
Contributor Author

tochwill commented Feb 3, 2020

Just need to update bundle size config

@tochwill
Copy link
Contributor Author

tochwill commented Feb 3, 2020

Fixing failing tests...

@tochwill tochwill removed the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Feb 3, 2020
@tochwill tochwill merged commit e685035 into latest Feb 4, 2020
@tochwill tochwill deleted the navigation-and-skip-link branch February 4, 2020 09:07
tochwill added a commit that referenced this pull request Feb 4, 2020
This reverts commit e685035, reversing
changes made to e93de56.
amywalkerdev added a commit that referenced this pull request Feb 4, 2020
Revert "Merge pull request #5234 from bbc/navigation-and-skip-link"
@tochwill tochwill restored the navigation-and-skip-link branch February 4, 2020 11:30
tochwill added a commit that referenced this pull request Feb 10, 2020
@sareh sareh deleted the navigation-and-skip-link branch February 21, 2020 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-priority ux To be reviewed by UX before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation Integration
9 participants