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.

Sign up for GitHub

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

Jump to bottom

ER-920 Mobile side menu #771

Merged
merged 10 commits into from
Apr 26, 2024
Merged

ER-920 Mobile side menu #771

merged 10 commits into from
Apr 26, 2024

Conversation

martikat
Copy link
Contributor

@martikat martikat commented Apr 11, 2024 •
edited
Loading

ER-920

This also includes the fix for ER-1009

@martikat martikat added this to the rc2.0.0 milestone Apr 11, 2024
@peterdavidhamilton
Copy link
Contributor

peterdavidhamilton commented Apr 19, 2024 •
edited by martikat
Loading

@martikat

  • entire stylesheets from the old design are still present and should be removed
  • the active links aren't needed and the burger menu is always visible so the use of a conditional is not appropriate for the new design
  • if there is a JS issue for the accordion action on the nav links let's confirm that on a prod deployment

@github-actions GitHub Actions
Copy link

https://hfeyp-review-pr-771.azurewebsites.net

@martikat martikat marked this pull request as ready for review April 24, 2024 15:31
peterdavidhamilton
peterdavidhamilton suggested changes Apr 25, 2024
View reviewed changes
app/models/page.rb Show resolved Hide resolved
app/components/header_component.html.slim Outdated Show resolved Hide resolved
app/assets/stylesheets/subnav.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/metadata.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/links.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/page-contents.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/other-resources.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/application.scss Outdated Show resolved Hide resolved
peterdavidhamilton
peterdavidhamilton suggested changes Apr 25, 2024
View reviewed changes
Copy link
Contributor

@peterdavidhamilton peterdavidhamilton left a comment

Choose a reason for hiding this comment

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

This npm package replaces this rubygem. Can you add it and see what it says?

app/assets/stylesheets/cta.scss Show resolved Hide resolved
app/assets/stylesheets/downloadable-media.scss Show resolved Hide resolved
@martikat martikat added this pull request to the merge queue Apr 26, 2024
Merged via the queue into main with commit 500e03f Apr 26, 2024
2 checks passed
@martikat martikat deleted the ER-920-mobile-side-menu branch April 29, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterdavidhamilton peterdavidhamilton peterdavidhamilton approved these changes

Assignees
No one assigned
Labels
review
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@martikat @peterdavidhamilton