-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add site title partial #118
Conversation
Pull Request Test Coverage Report for Build 7980315778Details
💛 - Coveralls |
8183b4a
to
e2d7575
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should just remove this now:
https://github.com/MITLibraries/timdex-ui/blob/main/app/views/layouts/_site_nav.html.erb#L3-L6
If we have a site title and an optional platform name in the header, the remaining bit in the "nav" seems redundant. The alternative would be something like only displaying this version if PLATFORM_NAME is set but that feels like a side effect and not something we really need to support.
Requested change: hd-2
class on non-Geo site title in the new partial.
The other bits I'm open for change or not depending on how you prefer.
<h1 class="hd-2">Search for Geographic/GIS data</h1> | ||
<p>Find GIS data held at MIT and other universities</p> | ||
<% else %> | ||
<h1>Search the MIT Libraries</h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We likely want to use the same class for non-GDT. hd-2
feels like a better size than our plain h1
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, good call. That was an oversight on my part.
5722689
to
d2c1c39
Compare
Why these changes are being introduced: The GeoData visual design includes a heading at the top of each page, along with some descriptive text. Relevant ticket(s): https://mitlibraries.atlassian.net/browse/GDT-182 How this addresses that need: This adds the requested element as a partial, with conditional copy for GDT versus non-GDT applications. It also moves the label text to the `title` attribute, as the label is is now visually redundant but may still be useful to screen reader users. Side effects of this change: * The input label before form submit is now the same as the placeholder text. This feels fine since screen readers might not read placeholder text, but they do consistently rely on labels. * The copy for non-GDT titles did not come from UXWS and is subject to change. * The conditional title-in-nav has been removed from `site_nav`. This seems superfluous since we now have h1 tags.
d2c1c39
to
6026751
Compare
Why these changes are being introduced:
The GeoData visual design includes a heading at the top of each
page, along with some descriptive text.
Relevant ticket(s):
https://mitlibraries.atlassian.net/browse/GDT-182
How this addresses that need:
This adds the requested element as a partial, with conditional
copy for GDT versus non-GDT applications. It also moves the label text
to the
title
attribute, as the label is is now visually redundant butmay still be useful to screen reader users.
Side effects of this change:
text. This feels fine since screen readers might not read placeholder
text, but they do consistently rely on labels.
to change.
Developer
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)
Code Reviewer
(not just this pull request message)
Requires database migrations?
NO
Includes new or updated dependencies?
NO