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.

Sign up for GitHub

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

Jump to bottom

[Dialog]: Dialog width calculation does not take Button Text Width into consideration #10012

Open
1 task done
wvudako opened this issue Oct 10, 2024 · 10 comments · May be fixed by #10993 or #11105
Open
1 task done

[Dialog]: Dialog width calculation does not take Button Text Width into consideration #10012

wvudako opened this issue Oct 10, 2024 · 10 comments · May be fixed by #10993 or #11105
Assignees
@hinzzx
Labels
bug This issue is a bug in the code TOPIC B

Comments

@wvudako
Copy link

wvudako commented Oct 10, 2024

Describe the bug

{ Dialog, Bar } from '@ui5/webcomponents-react';

Source Doc:
Dialog: https://sap.github.io/ui5-webcomponents-react/v2/?path=/docs/modals-popovers-dialog--docs
Dialog Styling with MessageViewButton:
https://experience.sap.com/wp-content/uploads/sites/56/2021/11/message-popover_overview_1.96.png
https://experience.sap.com/fiori-design-web/messaging/#message-popover

Using a dialog as described in your documentation with a small content and long Button Text, the Button Text is truncated.
This is even more of an issue if as described in the linked Fiori Doc you have a Save and a Close Button as well as Space for the MessageViewButton.
In the stackblitz as well as in your storybook the dialog has a min-width of 20rem, which our local implementation does not get automatically, therefore amplifying the issue. Setting that value was our fix for now as well

I am very sure that this behaviour has changed from webcomponents 2.2 to 2.3 (maybe the min-width got lost), but we can not easily verify that. Either way this is likely a webcomponents issue

      <Dialog
        footer={
          <Bar
            design="Footer"
            endContent={
              <Button onClick={() => setOpen(false)}>
                This is a very long text that exceeds the calculated width of
                the dialog
              </Button>
            }
          />
        }
        headerText="Dialog Header"
        open={open}
      />

Isolated Example

https://stackblitz.com/edit/github-lqbaqm

Reproduction steps

  1. https://stackblitz.com/edit/github-lqbaqm
  2. Click Button -> Dialog opens
  3. Text is truncated

Expected Behaviour

(Up to a reasonable max-width) the content of the footer is taking into consideration when calculating the dialog width

Screenshots or Videos

No response

UI5 Web Components for React Version

2.2.0

UI5 Web Components Version

2.3.0

Browser

Chrome

Operating System

Windows 11

Additional Context

No response

Relevant log output

No response

Organization

No response

Declaration

  • I’m not disclosing any internal or sensitive information.
@wvudako wvudako added the bug This issue is a bug in the code label Oct 10, 2024
@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 14, 2024
@PetyaMarkovaBogdanova PetyaMarkovaBogdanova self-assigned this Oct 14, 2024
@PetyaMarkovaBogdanova
Copy link
Contributor

Hi team,
I noticed that in current implementation the Dialog footer uses ui5-toolbar, while in the prepared use-case it is ui5-bar.
Can you, please, check where the difference in the max-width of the button is coming from?
Best regards,
Petya Markova.
(UI5 Dispatcher)

@wvudako
Copy link
Author

wvudako commented Oct 14, 2024

Hi. The given code is from your sourcecode in
https://sap.github.io/ui5-webcomponents-react/v2/?path=/docs/modals-popovers-dialog--docs#example

But using the new Toolbar instead of a Bar leads to moving the action into an overflow menu instead of cropping
image

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova removed their assignment Oct 14, 2024
@PetyaMarkovaBogdanova
Copy link
Contributor

Hi @gitgdako ,
I assigned the issue to the involved team and the handover message was targeting them.
They will take a look at the case and get back to you ASAP.
Best regards,
Petya Markova.
(UI5 Dispatcher)

@LidiyaGeorgieva LidiyaGeorgieva self-assigned this Oct 15, 2024
@LidiyaGeorgieva
Copy link
Contributor

Hello @gitgdako ,

Please be more specific with the reported issue.
In the isolated example the text of the button is truncated, but the dialog is with min-width 20rem:
{47551229-4694-4E42-94CE-B75D83D4CA3C}
In the description you mention: "In the stackblitz as well as in your storybook the dialog has a min-width of 20rem, which our local implementation does not get automatically, therefore amplifying the issue. Setting that value was our fix for now as well".

What is the reported issue - the missing of min-width 20rem or the truncation of the button text (when there is min-width 20rem)?

Regards,
Lidiya

@wvudako
Copy link
Author

wvudako commented Oct 30, 2024

As stated

Expected Behaviour
(Up to a reasonable max-width) the content of the footer is taking into consideration when calculating the dialog width

I would expect the text not to be truncated to a reasonable max width... or not to be truncated ever and leave it to the dev to define a maxwidth. Just setting the min-width automatically is a dirty fix imo and should just be the solution in case you dont want to put too much time into this. Especially needing to override in case you want a smaller dialog is not pretty

@LidiyaGeorgieva
Copy link
Contributor

LidiyaGeorgieva commented Nov 4, 2024 •
edited
Loading

Hello @gitgdako ,

The ui5-bar is with width: 100%, which is 100% of the width of the ui5-dialog (which if not specified is the default min width).
In your case the solution is to set width to the ui5-bar, which will be sufficient for the button text to be fully shown.
For example:
{C5FF3E2C-0355-478E-996B-43E4B3E3E9D1}

And the result:
{364B8E3F-068F-4A7D-B487-824AFF6825AC}

Best Regards,
Lidiya

@github-project-automation github-project-automation bot moved this from New Issues to Completed in Maintenance - Topic RD Nov 4, 2024
@wvudako
Copy link
Author

wvudako commented Nov 4, 2024

I explicitly stated that I do NOT want to specify the width for every single Dialog. UI5 is pretty good in calculating necessary sizes and for the specified buttons this was working well in 1.x
Having to set the width of every single Dialog Footer considering styling (and any changes in the future) and any dynamic components within would be a major downgrade

@github-project-automation github-project-automation bot moved this from Completed to In Progress in Maintenance - Topic RD Nov 5, 2024
@dimovpetar dimovpetar self-assigned this Nov 5, 2024
@dimovpetar
Copy link
Contributor

dimovpetar commented Nov 5, 2024 •
edited
Loading

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

Looks like the ui5-bar is a query container now. Its root element has container-type: size; style. This makes it ignore its children width requirements, therefore, now when put inside ui5-dialog, or in other similar container, there is high chance that children will be clipped. This behavior is described in detail here https://developer.mozilla.org/en-US/docs/Web/CSS/container-type.

We have had similar problems in another component. Based on that experience, I see 2 possible solutions:

  1. make the ui5-bar consider its children width as before. Compare v1 example to v2 example.
  2. Inform app developers that they are now responsible to set the ui5-bar width.

Best regards,
Petar

@hinzzx hinzzx self-assigned this Feb 28, 2025
@hinzzx hinzzx moved this from New Issues to In Progress in Maintenance - Topic B Feb 28, 2025
@hinzzx hinzzx linked a pull request Mar 4, 2025 that will close this issue
Open
@hinzzx
Copy link
Contributor

hinzzx commented Mar 4, 2025

Hi @wvudako ,

After a discussion with our architect, a solution was raised. The default container-type of the ui5-bar will remain the same, yet it would be able to be easily overwritten. After the PR is merged you would be able to remove the container-type of all bars inside ui5-dialog in just three lines of code:

[ui5-dialog] [ui5-bar][slot="footer"] {
        container-type: unset;
    }

Best Regards,
Stoyan

@tsanislavgatev tsanislavgatev linked a pull request Mar 14, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hinzzx hinzzx

Labels
bug This issue is a bug in the code TOPIC B
Projects
Maintenance - Topic B
Status: In Progress
Milestone
No milestone
6 participants
@LidiyaGeorgieva @Lukas742 @PetyaMarkovaBogdanova @dimovpetar @hinzzx @wvudako