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

Refinement for the New Feature Announcement area #1579

Merged
merged 3 commits into from
Mar 3, 2022
Merged

Conversation

Vinnl
Copy link
Collaborator

@Vinnl Vinnl commented Mar 1, 2022

This PR fixes a couple of issues identified in #1561. Specifically:

You'll want to add an extra rule to remove this for the last item.

#1561 (comment)

Done in c9a159c.

I can't seem to close the tab when clicking the news button again.

#1561 (review)

Done in a967d58.

Kooha-03-01-2022-14-32-10.mp4

It also fixes #1583.

As for

Another thing is that when I shrink the device width, the panel gets cut off at a certain breakpoint (around 762px). Are we keeping the whats new feature on mobile as well?

Since it switches to a drop-down menu on mobile, I decided to just not show it there for now, since that interaction (a drop-down in a drop-down) can get very gnarly very quickly. It's there in #1562, though that might at some point need to transition to a drop-down as well.

How to test:

  • l10n dependencies have been merged, if any.
  • I've added a unit test to test for potential regressions of this bug (or this is a front-end change, where we don't yet have unit test infrastructure).
  • I've added or updated relevant docs in the docs/ directory.
  • All UI revisions follow the coding standards, and use Protocol tokens where applicable (see /static/scss/libs/protocol/css/includes/tokens/dist/index.scss).
  • Commits in this PR are minimal and have descriptive commit messages.

@Vinnl Vinnl requested review from maxxcrawford and codemist March 1, 2022 13:37
@Vinnl Vinnl self-assigned this Mar 1, 2022
@Vinnl
Copy link
Collaborator Author

Vinnl commented Mar 2, 2022

Added 8ad30eb to fix #1583 as well.

Copy link
Collaborator

@codemist codemist left a comment

Choose a reason for hiding this comment

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

LGTM - Wondering if we should make it closeable by clicking outside of the panel too.

@Vinnl
Copy link
Collaborator Author

Vinnl commented Mar 3, 2022

@codemist That is already the case! (That's what window.addEventListener("click", close); does - it adds an event listener for clicking anywhere in the window.)

@Vinnl Vinnl merged commit f86b94b into main Mar 3, 2022
@Vinnl Vinnl deleted the new-stuff-followup branch March 3, 2022 16:47
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.

The “News” menu cannot be closed by clicking on it a second time or by pressing the Esc key
2 participants