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

Dialog: Dialog content alignment issue with zoom #10000

Closed
1 task done
saurabhmehta02 opened this issue Oct 9, 2024 · 15 comments · Fixed by #10199
Closed
1 task done

Dialog: Dialog content alignment issue with zoom #10000

saurabhmehta02 opened this issue Oct 9, 2024 · 15 comments · Fixed by #10199
Assignees
Labels

Comments

@saurabhmehta02
Copy link

Describe the bug

We have developed UI components using the web component and these are also used in mobile app.
In mobile devices for accessibility purpose, user can use dynamic font size setting and increase the font size.
If we increase the base font by 1.5x or 2x (base font size at html root is 24px or 32px) then web components sets left position in negative in mobile view.
For example:
if base font size is 30px then styles becomes: left: -85px
if base font size is 32px then styles becomes: left: -105px

Because of this left style, the dialog content isn't aligned in mobile resolution with higher font size.

Isolated Example

No response

Reproduction steps

Navigate to https://sap.github.io/ui5-webcomponents-react/v1/?path=/story/modals-popovers-dialog--with-content&args=stretch:!true
Open chrome dev tools and enable mobile view
Set base font size to above 24px or higher and you can notice inline style is added at the dialog level and because of which the dialog content isn't aligned

Expected Behaviour

No response

Screenshots or Videos

No response

UI5 Web Components for React Version

1.24.0

UI5 Web Components Version

1.21.0

Browser

Chrome, Safari

Operating System

iOS and Android

Additional Context

No response

Relevant log output

No response

Organization

SAP Labs

Declaration

  • I’m not disclosing any internal or sensitive information.
@saurabhmehta02 saurabhmehta02 added the bug This issue is a bug in the code label Oct 9, 2024
@girdharpatel
Copy link

Fixing this issue is very important for accessible users who are using mobile devices.

@Lukas742
Copy link
Collaborator

Thanks for reporting! I'll forward this issue to our UI5 Web Components Colleagues as the affected component is developed in their repository.

@Lukas742 Lukas742 transferred this issue from SAP/ui5-webcomponents-react Oct 10, 2024
@NHristov-sap
Copy link
Contributor

Hello @SAP/ui5-webcomponents-topic-rd,

Please check if this is valid use case.

Best Regards,
Nikolay Hristov

@LidiyaGeorgieva
Copy link
Contributor

Hello @saurabhmehta02 ,

Kindly provide more detailed steps for reproducing the issue along with a screenshot. Also, there seems to be a discrepancy between the title and the description: the title mentions "zoom," while the description refers to "Set base font size to above 24px or higher..."

Best Regards,
Lidiya

@saurabhmehta02
Copy link
Author

Dialog rendering when base font size is 16px:
image

Dialog rendering when base font size is 28px:
image

Left position value:
image

Dialog rendering when base font size is 32px:
image

@LidiyaGeorgieva I hope above screenshots are enough for you to understand the issue.
Also, its easily reproducible in the web components demo too as shared in the reproduce steps.

We are using the web module creating using the dialog component in mobile app, its zoom in the mobile device (for accessible users). The font size in the web module is adjusted dynamically according to the zoom level on the mobile device.

@LidiyaGeorgieva
Copy link
Contributor

Hello @saurabhmehta02 ,

Please be more specific how you "Set base font size to above 24px or higher"?
If I set "font-size: 30px" to the HTML element I see no issue:
{3D298A13-DD32-454F-B411-FCF2527B6D9E}

Best Regards,
Lidiya

@saurabhmehta02
Copy link
Author

Hi @LidiyaGeorgieva
Setting font-size here in the dev tools won't reflect for all the components.
I am updating font size at the html root using below code in JavaScript:
document.documentElement.style.fontSize = "32px";

As I mentioned previously, in our use case the web module is being used in the mobile app too.
When user is changing font-size in the mobile device, we set font-size for the web module as well using the code I shared.
This will ensure the base font size is changed for all the elements.

Thanks,
Saurabh

@saurabhmehta02
Copy link
Author

@LidiyaGeorgieva If you want to reproduce in the component demo page, please use dev tools and open the dialog component in mobile view (which have done already) and then in the console run following script:
document.documentElement.style.fontSize = "32px";

Thanks,
Saurabh

@LidiyaGeorgieva
Copy link
Contributor

Hello @saurabhmehta02 ,

After following your steps, the dialog appears fine and I don't see any issues:
{5C4EA8DB-BD90-4D59-8EC2-3A79458E7678}

Please provide an isolated, reproducible example of the problem, or we’ll need to close the ticket.

Best Regards,
Lidiya

@saurabhmehta02
Copy link
Author

@LidiyaGeorgieva I am attaching the sample react app and two recordings.
In first recording, I am sharing steps to reproduce the issue in the web component demo page itself.
In the 2nd recording, I have reproduced the issue in the sample react app.

Steps to run the app:
node version 18.x.x
npm install --legacy-peer-deps
npm run start

It should ideally run on port 3000 so URL should be localhost:3000.
sample_repo.zip

Recording2.mov
Issue_recording.mov

LidiyaGeorgieva added a commit that referenced this issue Nov 15, 2024
Issue: When the base font in the 'html' element is changed to something
different than the default 16px (for example 32px) the width
of the dialog can become larger than the width of the phone's
display. This is caused by the 'min-width' (20rem) of the dialog.
For the dialog on phone it is recommended by the design to set the dialog's
property 'stretch' to true to use the full screen size.

The solution: When we have stretched dialog on phone the 'min-width' should
 not be applied (the width is 100%).

fixes: #10000
@github-project-automation github-project-automation bot moved this from New Issues to Completed in Maintenance - Topic RD Nov 19, 2024
@ui5-webcomponents-bot
Copy link
Collaborator

🎉 This issue has been resolved in version v2.5.0-rc.1 🎉

The release is available on v2.5.0-rc.1

Your semantic-release bot 📦🚀

LidiyaGeorgieva added a commit that referenced this issue Nov 26, 2024
…10199)

Issue: When the base font in the 'html' element is changed to something
different than the default 16px (for example 32px) the width
of the dialog can become larger than the width of the phone's
display. This is caused by the 'min-width' (20rem) of the dialog.
For the dialog on phone it is recommended by the design to set the dialog's
property 'stretch' to true to use the full screen size.

The solution: When we have stretched dialog on phone the 'min-width' should
 not be applied (the width is 100%).

fixes: #10000
downport of #10199
LidiyaGeorgieva added a commit that referenced this issue Dec 2, 2024
…10255)

Issue: When the base font in the 'html' element is changed to something
different than the default 16px (for example 32px) the width
of the dialog can become larger than the width of the phone's
display. This is caused by the 'min-width' (20rem) of the dialog.
For the dialog on phone it is recommended by the design to set the dialog's
property 'stretch' to true to use the full screen size.

The solution: When we have stretched dialog on phone the 'min-width' should
 not be applied (the width is 100%).

fixes: #10000
downport of #10199
@saurabhmehta02
Copy link
Author

Hi @LidiyaGeorgieva
We use react web components in our project - https://sap.github.io/ui5-webcomponents-react
Has the fix been delivered for react web components as well?
Kindly update.

Thanks,
Saurabh

@LidiyaGeorgieva
Copy link
Contributor

@Lukas742 , could you help here?
The fix is included in version v2.5.0-rc.1 of ui5-webcomponents

@Lukas742
Copy link
Collaborator

Lukas742 commented Dec 4, 2024

Hi @saurabhmehta02

Starting with version 2.4.0 of @ui5/webcomponents-react, all ui5wcr packages now align their minor version with @ui5/webcomponents. However, pre-release versions, such as the current release candidate, are not supported out of the box. This is because when a new feature (like a component, property, etc.) is added, our wrapper needs to be updated as well.

For bug fixes, you can likely still test the RC version, but new features that require "wrapping" won't be available as React props, components, etc., and you will most likely encounter peer-dependency errors.

@ui5-webcomponents-bot
Copy link
Collaborator

🎉 This issue has been resolved in version v1.24.14 🎉

The release is available on v1.24.14

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

6 participants