-
Notifications
You must be signed in to change notification settings - Fork 82
Reactions: Hide title more comprehensively #1709
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
Conversation
@jeherve Was hiding the new block heading what you were referring to in the previous ticket? If so, I misunderstood. |
Co-authored-by: Matthias Pfefferle <pfefferle@users.noreply.github.com>
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 think we need to add a testing infrastructure for these blocks to prevent these regressions going forward. |
@pfefferle Have you had a chance to test this yet? |
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 seems to work well now, with your fix. 🚢
Follow-up to #1705.
While #1705 fixed titles for legacy Reactions blocks (v1.0.0), it did not account for hiding titles added through heading blocks, when there are no reactions.
In this PR, we standardize around the v2.0.0 markup with fallbacks for v1.0.0 titles, and then add a
hidden
attribute when there are no reactions.See https://mastodon.social/@vsg_DE/114512927652473093
Proposed changes:
view.js
.hidden
attribute to the title when there are no reactions.Other information:
Testing instructions:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Titles added through a Heading block in the Reactions block now stay properly hidden when there are no reactions.