Wrap long dataset name in success modal

getodk / central-frontend

Vue.js based frontend for ODK Central

https://docs.getodk.org/central-intro/

Apache License 2.0

32 stars 56 forks source link

Wrap long dataset name in success modal #968

Closed ktuite closed 4 months ago

ktuite commented 4 months ago

Closes https://github.com/getodk/central/issues/630

I feel kind of unsure about these CSS changes. I did add overflow-wrap: break-word; to the text-block mixin, and then used that mixin.

I also tried using a flexbox, but ran into an issue where the text flex item was hanging off the edge. Wasn't sure how to fix it, so that's where the 80% width came from...

What has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Does this change require updates to user documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

[ ] run npm run test and npm run lint and confirmed all checks still pass OR confirm CircleCI build passes
[ ] verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

matthew-white commented 4 months ago

I also tried using a flexbox, but ran into an issue where the text flex item was hanging off the edge. Wasn't sure how to fix it, so that's where the 80% width came from...

Hmm that's interesting that it hangs off the edge. Two ideas:

Specify flex-shrink: 0; for the .icon-check-circle. Specify width: 100%; for the <div>.
Specify flex-shrink: 0; for the .icon-check-circle. Don't specify any width for the <div>, but do specify flex-grow: 1;.

If there's a way to use the full width available, that sounds nice, especially for languages where the text might be longer. But 80% also sounds completely fine if that's what gets things working.

I'll also add a few more CSS thoughts below.

matthew-white commented 4 months ago

I'm thinking that there will be a similar issue in FieldKeyNew. If it's not too hard, do you think that could be fixed at the same time? We could also file an issue about it and circle back to it later.

ktuite commented 4 months ago

I think the issue is with overflow-wrap: break-word... when I tried overflow-wrap: anywhere, it could fit the text in the box without hanging over.

Was looking at this, maybe it is relevant?

The difference between anywhere and break-word is apparent when calculating the min-content intrinsic sizes. With break-word, the browser doesn’t consider the soft wrap opportunities introduced by the word break when calculating min-content intrinsic sizes, but it does with anywhere. For more about min-content intrinsic sizes, check out our guide here.

I ended up going with the width: 80% change because even with the above clues, I wasn't able to get past the text hanging out of its box.