Skip to content

add padding bottom to avoid overlap with banners #2454

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

Closed
wants to merge 14 commits into from

Conversation

jomcarvajal
Copy link
Contributor

@jomcarvajal jomcarvajal commented Mar 31, 2025

CORE-802

The issue more than resize text is related with overlap text in all resolutions. Instead of only focus on small resolutions, I added padding according resolutions. The padding is set by the number of toast messages in queu and a constant value taken from an aprox height of those toastBanners

Screen.Recording.2025-03-31.at.1.50.50.PM.mov

@jomcarvajal jomcarvajal requested a review from a team as a code owner March 31, 2025 20:43
@jomcarvajal jomcarvajal requested a review from RoyEJohnson March 31, 2025 20:43
@TomWoodward TomWoodward temporarily deployed to rex-web-core-802-conten-ymw96p March 31, 2025 20:43 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-core-802-conten-ymw96p March 31, 2025 20:44 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-core-802-conten-ymw96p March 31, 2025 20:47 Inactive
@@ -63,7 +78,12 @@ const PageToasts = (props: ToastProps | {}) => {
const mobileToolbarOpen = useSelector(mobileToolbarOpenSelector);

return (
<ToastContainerWrapper role='alertdialog' {...props} mobileToolbarOpen={mobileToolbarOpen}>
<ToastContainerWrapper
role='alertdialog'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that with the role "alertdialog" it does not matter that the content overlaps.
It was previously "alert". The difference is that "alertdialog" is something that is supposed to be dismissed -- like these. That role change was a change I made recently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should cancel the card CORE-802 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be reviewed along with CORE-804 (the card my PR was attached to). I think they're effectively the same.
I don't think it's wrong to make room for the boxes, but I think if we want to do that, we might just want to have them appear in a reserved space rather than absolute-positioned.

@TomWoodward TomWoodward temporarily deployed to rex-web-core-802-conten-ymw96p March 31, 2025 20:58 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-core-802-conten-ymw96p April 3, 2025 15:58 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-core-802-conten-ymw96p April 4, 2025 14:04 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-core-802-conten-ymw96p April 8, 2025 16:25 Inactive
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