[2.x] fix(approval): scope the unapproved fade to text, not the whole post - #4836
[2.x] fix(approval): scope the unapproved fade to text, not the whole post#4836karl-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
imorland
left a comment
imorland
left a comment
There was a problem hiding this comment.
The stacking-context diagnosis is correct and the issue history is accurate — but I don't think this is the right fix, and it'll need reworking before it can go in.
The uniform 50% grey is a deliberate design choice. A pending post is meant to read as de-emphasised in its entirety — avatar, actions and all — while staying legible enough for a moderator to judge it. This PR restores the avatar, Reply and Like to full strength to free the dropdown, which isn't really a side effect — it's the mechanism of the fix, and it works by undoing that design. I'd like to keep the greying uniform.
The .Post--hidden comparison doesn't quite hold. .Post--hidden collapses the post — display: none on body/footer/side when :not(.revealContent), header muted via color, opacity only in the revealed state. That's "hide unless revealed". An unapproved post needs to stay fully shown and simply marked pending, so reusing those selectors borrows styling built for the opposite intent — it reads as precedent but isn't quite one.
The underlying bug is still there. The root cause is that opacity < 1 on the <article> traps every absolutely-positioned popup inside it — the controls dropdown, the user card, the post-meta modal. #4453 freed the user card and trapped controls; this frees controls at the cost of the grey. It's the same pendulum swung the other way rather than a resolution.
I think the fix wants to keep the whole-post fade and address the popups where they render: lift the controls dropdown, user card and post-meta modal out of the post's stacking context so the article can keep opacity: 0.5 intact. That scopes the change to the popup that's open, which is the only surface the bug is actually on — and it'd properly close out the long-standing issue rather than moving it again. Happy to help think through the portal/floating side of that if it's useful.
2 participants
.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