argoproj / argo-ui

Argoproj shared React components
Apache License 2.0
220 stars 178 forks source link

fix(notification): fit long words #546

Closed JessieTeng89 closed 2 months ago

JessieTeng89 commented 5 months ago

Fixes https://github.com/argoproj/argo-ui/issues/512 It is a layout issue when there is a long word in the error message toast popup, so added a style in that component to fix the issue.

Screenshots: before fix:

image

after fix:

image
JessieTeng89 commented 4 months ago

@crenshaw-dev Kindly help to review this PR

JessieTeng89 commented 4 months ago

Hi @crenshaw-dev @jmeridth @terrytangyuan @rbreeze Much appreciate if you can help to review it or let me know if any other actions needed before merging it? Thanks!

JessieTeng89 commented 3 months ago

Hi @agilgur5 A gentle follow up with this PR, could you please help to review it?

agilgur5 commented 2 months ago

Yes, I had been planning to; I'm not sure why you asked several other uninvolved people for review a day after you made changes, which was a full 2 weeks after changes were requested.

Please be patient, especially if you yourself do not respond immediately -- it would not be surprising if a maintainer took as long as you did to respond if not longer -- I don't think it's reasonable to expect maintainers to be faster than you yourself. And please interact with maintainers who already reviewed your PR, instead of asking others, which is counter-productive.

JessieTeng89 commented 2 months ago

Hi @agilgur5 I just updated the change based on your suggestion, sorry for the delay and Thanks for your support!

gillesbouvier-qz commented 1 month ago

Did this change add the following CSS?

.Toastify__toast-body > div:last-child {
  word-break: break-word;
  ...
}

If so, it is not working as expected as it is breaking short words too. Did you mean to use this instead?

    overflow-wrap: break-word;

Note: In contrast to word-break, overflow-wrap will only create a break if an entire word cannot be placed on its own line without overflowing.

agilgur5 commented 1 month ago

No, you can see the diff for yourself here in the PR, which is all of 4 LoC of an in-line style that uses overflow-wrap: break-word.

word-break: break-word is also deprecated, which I pointed out in my review of the very first iteration