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

Redesign local nav for GDT #113

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Redesign local nav for GDT #113

merged 1 commit into from
Feb 9, 2024

Conversation

jazairi
Copy link
Contributor

@jazairi jazairi commented Feb 7, 2024

Why these changes are being introduced:

The Geodata app requires links to GIS resources at MIT in the local nav. It also no longer requires the app name in the local nav, as GDT-125 moved this to the main header.

Relevant ticket(s):

How this addresses that need:

This updates the local nav as specified, provided the GDT feature is enabled.

Side effects of this change:

We will likely want to remove the app name from the local nav in all our applications and use the new method provided by the theme gem instead. However, it is out of scope of this ticket (and project) to make that decision, so I've left it in for now.

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

@mitlib mitlib temporarily deployed to timdex-ui-pi-gdt-125-re-lwf4lf February 7, 2024 22:37 Inactive
@coveralls
Copy link

coveralls commented Feb 7, 2024

Pull Request Test Coverage Report for Build 7849907042

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 99.444%

Totals Coverage Status
Change from base Build 7848267780: 0.0%
Covered Lines: 358
Relevant Lines: 360

💛 - Coveralls

@jazairi jazairi force-pushed the gdt-125-redesign-local-nav branch from 46f6875 to 28288fe Compare February 7, 2024 22:38
@jazairi jazairi temporarily deployed to timdex-ui-pi-gdt-125-re-lwf4lf February 7, 2024 22:38 Inactive
app/views/layouts/_site_nav.html.erb Outdated Show resolved Hide resolved
@jazairi jazairi force-pushed the gdt-125-redesign-local-nav branch from 28288fe to 97f1858 Compare February 8, 2024 19:23
@jazairi jazairi temporarily deployed to timdex-ui-pi-gdt-125-re-lwf4lf February 8, 2024 19:23 Inactive
@JPrevost
Copy link
Member

JPrevost commented Feb 9, 2024

@jazairi Just confirming this is still good with the latest change and can merge whenever you are ready along with the related PR

@jazairi
Copy link
Contributor Author

jazairi commented Feb 9, 2024

Thanks @JPrevost! Just waiting on QA before merging.

@jazairi jazairi temporarily deployed to timdex-ui-pi-gdt-125-re-lwf4lf February 9, 2024 19:05 Inactive
@jazairi jazairi requested a review from JPrevost February 9, 2024 19:06
@jazairi
Copy link
Contributor Author

jazairi commented Feb 9, 2024

@JPrevost Just pushed a couple of small changes from QA. I also added the nav title back in (conditionally) to stay consistent with how we plan to manage that for the time being.

@jazairi jazairi temporarily deployed to timdex-ui-pi-gdt-125-re-lwf4lf February 9, 2024 21:28 Inactive
Why these changes are being introduced:

The Geodata app requires links to GIS resources at MIT in the local
nav. It also no longer requires the app name in the local nav, as
GDT-125 moved this to the main header.

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/GDT-125
* https://mitlibraries.atlassian.net/browse/GDT-124

How this addresses that need:

This restyles the local nav as specified, provided the GDT feature
is enabled. It also makes the title in the nav conditional on whether
the `PLATFORM_NAME` environment variable is present. (This toggles the
name in the header.)

Side effects of this change:

None.
@jazairi jazairi force-pushed the gdt-125-redesign-local-nav branch from 18e3296 to e927c36 Compare February 9, 2024 21:39
@jazairi jazairi temporarily deployed to timdex-ui-pi-gdt-125-re-lwf4lf February 9, 2024 21:39 Inactive
@jazairi jazairi merged commit 0c18643 into main Feb 9, 2024
5 checks passed
@jazairi jazairi deleted the gdt-125-redesign-local-nav branch February 9, 2024 21:40
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.

4 participants