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

[BUG] - Template lines of MGT-Person component no longer show ellipsis #2975

Closed
JHolah opened this issue Jan 26, 2024 · 5 comments · Fixed by #2999
Closed

[BUG] - Template lines of MGT-Person component no longer show ellipsis #2975

JHolah opened this issue Jan 26, 2024 · 5 comments · Fixed by #2999
Labels
bug Something isn't working State: In Review
Milestone

Comments

@JHolah
Copy link

JHolah commented Jan 26, 2024

Describe the bug
MGT v3 has changed the styles of the template lines in the Person component so that text overflow is handled differently. There is currently no workaround or exposed CSS property to reinstate the styles.
The previous version enabled an ellipsis for any overflowing text in the details area, limiting it to a single line. The new version allows text to wrap to a new line. In our situation, this breaks the UI of several components in which the MGT Person component is used.

Steps to reproduce the behavior:

  1. Go to Person component on MGT Playground: https://mgt.dev/?path=/story/components-mgt-person--person
  2. Paste the following code into the HTML panel:
<div style="width:150px">
<mgt-person person-query="Debra Berger" view="threeLines"></mgt-person>
</div>

Note that details text wraps to a new line where previously it was clamped to one line with an ellipsis.

image

Expected behavior
Any details line which exceeds the available space should be hidden, with text-overflow: ellipsis

It appears that the following CSS properties that exist in the previous version, are no longer applied:

:host .person-root .details > div, mgt-person .person-root .details > div {
    text-overflow: ellipsis;
    white-space: nowrap;
    overflow: hidden;
}

Screenshots
The previous version limited text to one line:
old

Environment (please complete the following information):

  • Chrome
  • React
  • SharePoint

Additional context
It may be that this is an intended change after all, but in our cases, it breaks a lot of things The need to have line overflow styles remains. Exposing CSS properties for this would also be an acceptable solution.

@JHolah JHolah added bug Something isn't working Needs: Triage 🔍 labels Jan 26, 2024
@JHolah JHolah changed the title [BUG] [BUG] - Template lines of MGT-Person component no longer show ellipsis Jan 26, 2024
@sebastienlevert
Copy link
Contributor

Thanks for sharing @JHolah! I agree with your analysis and this was missed when moving to Fluent UI. We'll be queuing this bug for our post-v4 release. If you have time / want to send a PR our way, feel free and we'll be happy to provide guidance and reviews!

@gavinbarron
Copy link
Member

gavinbarron commented Jan 26, 2024

Hi @JHolah, this change was deliberate and driven out of accessibility requirements to not truncate visible text, this creates a disparity of experience between sighted users and those using screen reading technology.
I can't find a specific defect for this particular change so I think we caught it during the re-design work and never had an issue for it. But here's a functionally equivalent issue: #2350

What we can do is ensure that you can target these elements via a css selector. using something like:

mgt-person::part(detail-line) {
    overflow: hidden;
    text-overflow: ellipsis;
    width: -webkit-fill-available; // or and explicit width
    white-space: nowrap;
}

Coupled with this you'd need to restrict the width of the detail-wrapper by setting the --person-details-wrapper-width css property if you want to use the -webkit-fill-available width option.

Would that allow you to meet your requirements?

@JHolah
Copy link
Author

JHolah commented Jan 28, 2024

Thanks, @gavinbarron, I will see where we get with that and report back here.

@gavinbarron
Copy link
Member

@JHolah we'll need to add a part attribute into the markup we write in the shadow DOM for this to work. Sounds like if we did so it would resolve your challenges so we'll go with that plan.

@JHolah
Copy link
Author

JHolah commented Jan 30, 2024

Thank you, @gavinbarron. I was struggling to get things to work this way so that's much appreciated. Given that this is an intended change for accessibility, we are happy to roll with the change. If there is a way to reinstate things as per your plan, then that's a bonus, but an accessibility-first approach makes the most sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working State: In Review
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants