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

Put ExternalEmbedRemoveBtn on top #5887

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Put ExternalEmbedRemoveBtn on top #5887

merged 1 commit into from
Oct 24, 2024

Conversation

estrattonbailey
Copy link
Member

Evidently wasn't high enough in certain contexts (video). This seem to work not for all embed types.

@arcalinea arcalinea temporarily deployed to video-remove - social-app PR #5887 October 21, 2024 15:40 — with Render Destroyed
@estrattonbailey estrattonbailey mentioned this pull request Oct 21, 2024
Copy link

Old size New size Diff
7.9 MB 7.9 MB 8 B (0.00%)

@@ -11,7 +11,7 @@ export function ExternalEmbedRemoveBtn({onRemove}: {onRemove: () => void}) {
const {_} = useLingui()

return (
<View style={[a.absolute, a.pt_sm, a.pr_sm, {top: 0, right: 0}]}>
<View style={[a.absolute, a.pt_sm, a.pr_sm, a.z_50, {top: 0, right: 0}]}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems like it would be better fixed at the usage site, no? we can't guess the necessary z-order here, but we can be more certain in the place that defines the actual ordering (and it's possible we can also fix this by changing the DOM order itself)

@gaearon gaearon merged commit fa4cca1 into main Oct 24, 2024
6 checks passed
@gaearon gaearon deleted the video-remove branch October 24, 2024 19:02
rshigg pushed a commit to rshigg/social-app that referenced this pull request Nov 2, 2024
rshigg pushed a commit to rshigg/social-app that referenced this pull request Nov 2, 2024
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.

3 participants