[2.x] fix(approval): scope the unapproved fade to text, not the whole post#4836
Open
karl-bullock wants to merge 1 commit into
Open
[2.x] fix(approval): scope the unapproved fade to text, not the whole post#4836karl-bullock wants to merge 1 commit into
karl-bullock wants to merge 1 commit into
Conversation
Putting opacity on `.Post--unapproved` itself gives the whole `<article>` a stacking context, so every popup inside the post is trapped in it: the controls dropdown, the post-meta modal and the user card all render at 50% and get painted over by whatever follows the post in the stream. On a moderated forum that makes the controls menu unclickable, since the clicks land on the post underneath. This is the same problem flarum#4453 fixed for the user card, just moved. Before that change the opacity sat on `.Post-header`, which trapped the card; moving it up to the article freed the card but swept `.Post-actions` into the fade for the first time. Follow what core already does for `.Post--hidden`: mute the header with `color` and fade only the text-bearing children, so nothing above the post body ends up inside a stacking context. Fixes flarum#4505
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.Post--unapprovedputsopacity: 0.5on the whole<article>, which gives the post its own stacking context. Everything that pops out of the post is then stuck inside it. The controls dropdown, the post-meta modal and the user card all render at half opacity, and anything that follows the post in the stream paints over them. The practical effect is that the controls menu on a pending post can't be clicked, because the clicks land on the post underneath.To reproduce, with Approval enabled:
is_approved = 0on its row in thepoststableThis is the same bug #4453 dealt with, moved rather than removed. Before that PR the opacity sat on
.Post-header, which trapped the user card. Moving it up to the article freed the card, but pulled.Post-actionsinto the fade for the first time. The wider version of this has been open as flarum/issue-archive#328 since 2016, with #950 and #3031 closed as duplicates of it, and #4212 closed by #4453.The change here does what core already does for
.Post--hidden: mute the header withcolorand fade only the children that hold text, so nothing that pops out of the post ends up inside a stacking context.Worth flagging that this does change how a pending post looks. The avatar, Reply and Like go back to full strength, because
.Post--hiddenfades neither.Post-actionsnor the avatar. The post still reads as pending from the muted header and the "Awaiting approval" label. If uniform greying matters more, say so and I'll look again, but those elements are the ones that have to stay outside the fade for the popups to behave.Checked on 2.0.0-rc.5 and 1.8.17, in Chromium and Firefox, against a comment post, a discussion's opening post, and an unapproved event post. All three popups go from half opacity to full in every case.
1.x has the same root cause but breaks differently. The controls menu is fine there, and it's the user card and post-meta modal that suffer. The equivalent change fixes it, so I'm happy to open a 1.x PR if that's wanted.
Fixes #4505