-
Notifications
You must be signed in to change notification settings - Fork 13
Conversation
@@ -97,6 +97,19 @@ | |||
To create a production bundle, use `npm run build` or `yarn build`. | |||
--> | |||
<script type="text/javascript"> | |||
var defaultLanguage = 'en'; | |||
function getLangFromTranslateElement(translateElement) { |
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 attempted to grab the language code directly off of the translation element, but this didn't always seem to match the selected language. However, the translation element includes a dictionary of code/language correspondences, and it's possible to reverse engineer the code from the assigned inner text of the google translate element.
newTab: true, | ||
}, | ||
{ | ||
href: `${InfoLink}/funding`, |
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.
The sample code we were sent by Nice and Serious didn't include link addresses in the footer code. Where the links also appeared in the header, I've used those links, and elsewhere I've used the existing address; in a few cases I've guessed at the address based on the general naming pattern. Because the OAR team wants screenshots of the website on staging quickly and many of the linked pages aren't live yet regardless, I think it's best to confirm/update the links as a follow-up if needed prior to moving to production.
import Button from '@material-ui/core/Button'; | ||
|
||
export default () => ( | ||
<div |
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 hide the Google translate element and use relative positioning to display the styled button under the component, allowing clicks events to hit the translate element when users click the visible button.
}} | ||
> | ||
<span | ||
className="skiptranslate" |
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.
"skiptranslate" here is extremely important. Without this class, Google translate attempts to translate this span, replacing the inner text (language code) with a element with a translation of the current language code from the previously selected language into the current language in cases where the code is a word in the previously selected language. This causes extremely buggy and hard-to-track behavior, such as 'EN' being replaced by 'ON' or 'OF' depending on the 'source' language. @lederer This is the source of the strange behavior we saw yesterday.
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.
More info here: facebook/react#11538
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.
Looking great. A few tweaks to get over the finish line:
Header
- Entire header should be
font-weight: 700
and-webkit-font-smoothing: antialiased
- Sub-menu content should be left aligned rather than centered.
- Our header is
84px
tall while N&S's codepen is80px
. - Change breakpoint from
960px
to1080px
bc the app header has more content than the info site header.- Alternatively, we can ask if we can move Resources into How It Works or About Us, or if we can rename "Register/Contribute" to "Register".
- We should take the liberty of removing the sidebar header in search mode, where it's redundant with main header, and changing the sidebar header bg color to OAR Purple in other modes, since the gradient doesn't work nicely with the global header bg.
Translate
- The bottom portion of the button, below the text, isn't receiving click events. I think there's some work needed to ensure the invisible (google translate) button has the precise footprint as the visible (custom) button.
- Can we inject some CSS to override the styling of the open translate menu, so it gets positioned and styled like the other sub-menus, per the designs? I can imagine this as a follow-up issue if needed.
Footer
- "Built by Azavea" should not be uppercased
- Entire footer should be
-webkit-font-smoothing: antialiased
. - Footer padding should be
1rem
Some updates from a51cfae:
|
Awesome. Thanks for the fixes. The purple looks right. Didn't realize translate menu was in an iframe. In that case I think you're right that we're stuck with it. Small set of additional tweaks: Header
Footer
|
I will take a look at this after the tweaks suggested by Scott are implemented. |
Thank you for taking another look! I've pushed up the additional tweaks to styling. |
Thanks for the update. One last thing (sorry! I overlooked it before):
|
@lederer Quick question - after updating the links, I feel like the 'Built by Azavea' link looks like it's misaligned. Do you agree, or am I imagining things? |
Yup, good catch. Looks like N&S is using |
Thanks for working through the details! |
@@ -535,7 +535,10 @@ export const EmbeddedMapInfoLink = 'https://info.openapparel.org/embedded-map'; | |||
// when the width is set to 100% | |||
export const minimum100PercentWidthEmbedHeight = '500px'; | |||
|
|||
export const InfoLink = 'https://info.openapparel.org'; | |||
export const InfoLink = |
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.
It occurred to me that we could better test the header links if we used the staging site as the info site root URL in development for now. This did reveal a couple of bugs that I fixed, and a couple of questions that we should get answered.
Questions: It looks like the Data and Technology links have been merged into one page on the staging info site. Should we update our code the reflect this? Also, the sample footer from the Codepen has a 'donate' button, but there doesn't appear to be a 'Donate' page on the info site - should this be removed?
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.
Since the content appears to be merged I do think it makes sense for us to also merge the Data and Technology links on our side to match.
I would defer the decision on the Donate button to the OAR team. For now I would leave it in place.
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.
Thank your for all your effort working through this update. All the links work as expected (we expect a few 404s for some of the detail pages right now) and I did not see any styling regressions in the search or list processing pages. 👍
The info website has been updated with a new header and footer. In order to keep users' experiences as similar as possible between the info site and the map site, updates the header and footer to match. A code sample was provided for the info website's header and footer, and wireframes were provided for the full-width view of the map site's header and of the info site's mobile header. As much consistency as possible with these wireframes has been used. Differences include adding authentication, translation, and legal links to the mobile navigation, and adding the API documentation link to the full-width footer.
The "About Processing" and "About Facility Claims" pages have been moved to the updated info site, and no longer need to be maintained in the map site. In addition, some navbar components are no longer used in the updated navbar.
b1a12ff
to
9d72f25
Compare
Thank you for reviewing! |
Overview
The info website has been updated with a new header, footer, and font. In
order to keep users' experiences as similar as possible between
the info site and the map site, updates the header, footer, and font to match.
A code sample was provided for the info website's header and footer,
and wireframes were provided for the full-width view of the map site's
header and of the info site's mobile header. As much consistency as
possible with these wireframes has been used. Differences include
adding authentication, translation, and legal links to the mobile
navigation, and adding the API documentation link to the full-width
footer.
The font has been updated to the new font.
Several components which are no longer needed following the redesign have been removed.
Connects #1489, #1471
Demo
Mobile
Web
Google Translate
Notes
Mobile Design
Web Design
Codepen
The wireframes didn't include the API documentation link currently in the header. Per discussion with Scott, we moved this link to the footer on web view and removed it in mobile view, under the assumption that generally API users will be using a desktop/laptop.
The mobile menu wireframes didn't include a mockup of the changes required for the differences between the info site and the map site. After discussion with Scott, I used similarly styling to the same links in the header for contribute and login links. I added the authentication menu as a separate menu, similarly to how we had it previously.
The cookie preferences, terms of service, and privacy policy links/buttons are in the footer on web, but we hide the footer on mobile. We added these links as a submenu called 'more' but should discuss this label with the OAR team (changes to the title of the submenu can be held off for future updates).
Testing Instructions
./scripts/server
and navigate to the website.Checklist
fixup!
commits have been squashed