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

Fix feedback widget positioning + refactor to use popover #12135

Closed
wants to merge 4 commits into from

Conversation

pettinarip
Copy link
Member

@pettinarip pettinarip commented Feb 8, 2024

image

Description

To fix the FeedbackWidget positioning issue + fix the horizontal overflow #12035 + improve a11y aspects since we were using a wrong component (AlertDialog), this PR switches the base component to a Popover.

Related Issue

#12035
fixes: #12184

…nts and to fix issues with positioning on a sticky positioned parent
Copy link

netlify bot commented Feb 8, 2024

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit a2823ae
🔍 Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/65cdd8d76121bd000845c6f8
😎 Deploy Preview https://deploy-preview-12135--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

const size = "3rem"
return (
<Button
ref={ref}
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed for the PopoverTrigger to work correctly.

position="absolute"
w={{ base: size, lg: isExpanded ? "15rem" : size }}
h={size}
bottom={{ base: `${bottomOffset + 1}rem`, lg: 4 }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Absolute position the button to the right inside the sticky container.

</FixedDot>

<AlertDialog
<Box position="sticky" bottom="0">
Copy link
Member Author

Choose a reason for hiding this comment

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

Keep it at the bottom of the screen.

py="4"
px="2"
>
<PopoverCloseButton position="absolute" top="2" right="4" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why, but I had to manually position the close button. In the Chakra examples it seems to be positioned automatically.

Copy link
Member

@corwintines corwintines left a comment

Choose a reason for hiding this comment

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

Nice, thanks @pettinarip

Noticing that on mobile, there is an issue with this overflowing

Screenshot 2024-02-14 at 1 47 55 PM

Ive created this issue for this: #12184

It currently exists still, but would be good to fix in this I think.

@pettinarip
Copy link
Member Author

@corwintines good catch! added a new variant to control the internal width of the popover.

@pettinarip
Copy link
Member Author

@corwintines closing this in favor of #10997.

@pettinarip pettinarip closed this Feb 15, 2024
@github-actions github-actions bot added the abandoned This has been abandoned or will not be implemented label Feb 15, 2024
@nhsz nhsz deleted the feedback-widget-popover branch March 26, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned This has been abandoned or will not be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feedback widget overflowing on Mobile
2 participants