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

Horizontal Navigation #30

Merged
merged 6 commits into from
Feb 19, 2021
Merged

Horizontal Navigation #30

merged 6 commits into from
Feb 19, 2021

Conversation

erinesullivan
Copy link
Collaborator

@erinesullivan erinesullivan commented Feb 11, 2021

Overview

As a part of getting ILLiad up and running on the site, the From Other Institutions (Interlibrary Loan) page needed to be added under Requests. This required horizontal navigation to be added (similar to what you see in Shelf), redirects to be made, and unit tests to be updated.

Horizontal Navigation

In an effort to keep programming DRY, I created the horizontal_nav view and model to help generate the secondary navigations for all pages that are under Shelf and Requests. Any changes that need to be made in that area can be done within that file.

Testing

  • Run docker-compose run web bundle exec rspec to make sure the tests pass. Try and break the tests to double-check.
  • Run npm run compile to make sure the styles compile successfully.
  • Check if http://localhost:4567/requests/ redirects you to http://localhost:4567/requests/um-library.
  • Check all Shelf and Requests pages to make sure nothing is broken.

- Created the From Other Institutions (Interlibrary Loan) page
- Added redirects for Requests pages
- Added testing for new and updated pages
- Updated `shelf_navigation` to be `horizontal_navigation`, making it more global since it's used in both Shelf and Requests
- Added the view to all /shelf/ and /requests/ pages.
- Deleted shelf_navigation and requests_navigation.
@erinesullivan erinesullivan added enhancement New feature or request help wanted Extra attention is needed labels Feb 11, 2021
@erinesullivan erinesullivan self-assigned this Feb 11, 2021
@erinesullivan erinesullivan removed the help wanted Extra attention is needed label Feb 17, 2021
@@ -0,0 +1,20 @@
<%
path = request.path_info
nav = HorizontalNav.for(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

I love this change! This makes it easy for the horizontal nav to be reused because of your careful reorganizations of concerns and related logic.

Fantastic work, Erin!

@erinesullivan erinesullivan merged commit 2cbc209 into main Feb 19, 2021
@erinesullivan erinesullivan deleted the requests-navigation branch February 19, 2021 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants