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

Refactor & style AnnouncementBox #945

Merged
merged 1 commit into from
Jul 22, 2019

Conversation

andycochran
Copy link
Contributor

  • Moves the AnnouncementBox component to the site header so it's available on all routes
  • Uses Ember Tooltips instead of custom action to toggle the announcement (added benefit of clicking anywhere on page to close it)
  • Adds what-input to prevent :focus outline styles showing for non-keyboard navigation
  • Adds some temporary overrides for Labs UI and Ember Tooltips (should be remove once this PR has been merged/published)
  • Updates the language of the announcement to be more friendly/positive

Copy link
Collaborator

@allthesignals allthesignals left a comment

Choose a reason for hiding this comment

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

We should make sure that what-input doesn't break fastboot pre-rendering. Because that happens Node-side, we can't always expect there to be a window object.

<p class="text-small">
Thank you for using ZoLa!
<br>—<a href="mailto:zola_gis@planning.nyc.gov"> NYC Planning Labs</a>
</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks great! Why keep this inside the component instead of passing it as a block from the invoking side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would work. I had the whole thing in the site header, without using a component at all. I hastily moved it into the component. Makes sense to use it as a block component with the content in the header.

@allthesignals allthesignals merged commit b8729b1 into perf-announcement Jul 22, 2019
@allthesignals
Copy link
Collaborator

allthesignals commented Jul 22, 2019

@andycochran

Adds what-input to prevent :focus outline styles showing for non-keyboard navigation

Can we address this in a separate issue?

@andycochran
Copy link
Contributor Author

The reason I added what-input into this PR instead of addressing separately is because the new item in the site menu is a <button> element (the others are <a>) and when you click it you get the focus style.

image

You can notice the focus style on other elements too (like the dropdown in the BBL Lookup).

@hannahkates hannahkates deleted the perf-announcement-ac branch August 22, 2019 16:33
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.

None yet

2 participants