Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
285 stars 76 forks source link

dropdown button with width="full" wider than containing block #5903

Closed AdelheidF closed 9 months ago

AdelheidF commented 1 year ago

Actual Behavior

image

The dropdown button is wider than its containing block if the button text has no dashes or spaces. Button uses width="full".

Expected Behavior

text is wrapped

Reproduction Sample

https://codepen.io/afreitag/pen/GRGPdwQ

Reproduction Steps

open codepen and observe. Red box displays block dimensions.

Reproduction Version

next.657

Relevant Info

No response

Regression?

Don't think so

Impact

No response

Esri team

ArcGIS Map Viewer

geospatialem commented 1 year ago

Applicable in the case mentioned above where text has no breaks, such as "BirdObservationCommentBirdObservationComment" and where "_" characters are present, such as "Bird_Observation_Comment_Bird_Observation_Comment".

image

AdelheidF commented 1 year ago

Could this be part of 1.0?

benelan commented 1 year ago

How smart do you want this to be; are you expecting it to understand word breaks such as, capitalization, underscores, periods, etc.

BirdObservationCommentBirdObservation Comment

Or just wrap on the overflow

BirdObservationCommentBirdObservationCom ment

AdelheidF commented 1 year ago

Or just wrap on the overflow

I think this is fine. Don't you do this already in other places like this?

benelan commented 1 year ago

I believe so. Although interestingly, it currently understands hyphens as word breaks and wraps there.

macandcheese commented 1 year ago

FWIW, it breaks in a different way when the button is not the dropdown-trigger:

Screen Shot 2022-12-08 at 1 32 07 PM
geospatialem commented 1 year ago

@AdelheidF We're putting this in a list for Franco when he returns, but due to staffing, this one might land in time for the February release. Will post an update a few weeks into 2023.

geospatialem commented 1 year ago

Seeking design expertise for the dropdown button, if wrapping and/or truncating the text should be considered.

Elijbet commented 1 year ago

Seeking design expertise for the dropdown button, if wrapping and/or truncating the text should be considered.

@SkyeSeitz I'm assuming we follow the button pattern in https://github.com/Esri/calcite-components/issues/5660 where we truncate long text on fixed-width components that should not grow in height? I would think this pattern should be consistent regardless of where the button is slotted, is that so?

ashetland commented 1 year ago

Seeking design expertise for the dropdown button, if wrapping and/or truncating the text should be considered.

@SkyeSeitz I'm assuming we follow the button pattern in #5660 where we truncate long text on fixed-width components that should not grow in height? I would think this pattern should be consistent regardless of where the button is slotted, is that so?

Yep, agreed here.

Elijbet commented 1 year ago

@ashetland @SkyeSeitz How would the tooltip with full button text for sighted users work in this scenario, where the button is slotted as a trigger to dropdown within a block per use-case image provided in the description above? Or this scenario. image

Elijbet commented 1 year ago

We caught a bit of a design issue with truncated text buttons in https://github.com/Esri/calcite-components/issues/5660 . Mainly that the initial purpose of the tooltip is to provide context beyond what the button text is. To avoid swaying the use of the tooltip in another direction and limiting its use, we decided we could use HTML label for truncated buttons to reveal the full text for sighted users and leave tooltip alone to provide additional context. Once the truncated button is merged, this issue will pick up from there to achieve this solution instead.

MicrosoftTeams-image

The conversation was between @macandcheese, @ashetland, and me in the teams. After updating @driskull, we thought the above https://github.com/Esri/calcite-components/pull/6747 could be used as an interim solution. So let's merge that and I'll pick this up after issue #5660 is merged as well. Once the modified button is merged this might just work with minor tweaks. https://github.com/Esri/calcite-components/issues/5660#issuecomment-1500581533

github-actions[bot] commented 1 year ago

Installed and assigned for verification.

geospatialem commented 1 year ago

Verified on master.

image

AdelheidF commented 1 year ago

What happened???

It was working with @esri/calcite-components@1.3.0-next.2

But it now longer works with @esri/calcite-components@1.3.0 or 1.3.1

geospatialem commented 1 year ago

This might be related to https://github.com/Esri/calcite-components/pull/6664, which was merged 4 days after the fix above.

@driskull @Elijbet Would you be able to take a peek to see if button inherits parent width as we expect it? Saw some additional discussion above, which mentioned some design conflicts with button from #5660.

driskull commented 1 year ago

@Elijbet can you take this? Is the screenshot test still working correctly? I wonder what happened.

Elijbet commented 1 year ago

@Elijbet can you take this? Is the screenshot test still working correctly? I wonder what happened.

Yep, I'll take a look. It's not supposed to wrap anymore based on the other related issue discussions. Let me see what is the issue.

Elijbet commented 1 year ago

What happened???

It was working with @Esri/calcite-components@1.3.0-next.2

But it now longer works with @Esri/calcite-components@1.3.0 or 1.3.1

This issue was verified as resolved after the interim solution to wrap the text was implemented, awaiting the design approach to the long text with no breaks. The design decision that followed was to truncate text on buttons with a set height and provide a tooltip with full text for sighted users. This was implemented on the button component, and its installation caused a regression in the dropdown with the nested button. The button text truncation works when the button width is contained within the container, in this nested use case though the width=“full” on the button doesn’t work.

6958 swapped display: flex with display: block on the dropdown host to allow for the button to extend width="full".

Note: This should be scheduled for a future planned breaking change release.

In the meantime, here's a workaround:

calcite-dropdown {
  display: block;
}
ethanbdev commented 1 year ago

Ran into this and just wanted to add that it still happens if there are spaces https://codepen.io/eborgen/pen/qBQmbgm?editors=1010

The workaround still works

geospatialem commented 11 months ago

Spike efforts of https://github.com/Esri/calcite-design-system/issues/7692 might have similar groupings/relationships .

Consider tackling the spike in coordination with the breaking change in the above issue.

jcfranco commented 10 months ago

Installed an update to dropdown on rc that will make it easier to set up truncation in the original use case. You'll need to set 100% width in the dropdown or change its display to block as @Elijbet's suggestion shows. We can't do this reliably for all configurations since it really depends on the way the HTML/CSS is structured. For example, this shows the same issue can be seen with block + button.

github-actions[bot] commented 10 months ago

Installed and assigned for verification.

geospatialem commented 10 months ago

Verified with v2.0.0-rc.2 when calcite-dropdown's width is set to 100% per Franco's comment above.

image

AdelheidF commented 9 months ago

I tested this fix now with the original (simple) codepen and it seems fixed with 2.0. But when I look at it in Map Viewer it's still not working.

I updated the codepen to display the issue in the right-side flow. It seems that if the calcite-blocks are surrounded by calcite-sortable-list it's still not working. If you comment out this element it displays correctly.

https://codepen.io/afreitag/pen/GRGPdwQ;

This workaround no longer works either

calcite-dropdown {
  display: block;
}
jcfranco commented 9 months ago

If you set calcite-sortable-list's display to block, it'll work.

Screenshot 2023-12-08 at 2 11 54 PM

AdelheidF commented 9 months ago

Nice, this works. I will use that.

Is this a workaround or the solution? Basically do you want me to close this issue again?

jcfranco commented 9 months ago

This would be the solution. The recent change to dropdown makes truncation easier to set up, but we can't handle all cases as it depends on the HTML structure.