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

Fix poor table column width due to breaking words #31473

Merged
merged 4 commits into from
Jun 24, 2024

Conversation

brechtvl
Copy link
Contributor

Caused by #31091


Before
before

After
after

Example markdown for testing:

| **Report** | **Commits in `main`**  | **Remarks** |
| -- | -- | -- |
| | https://projects.blender.org/blender/blender/commit/e3a02b9b025282a46e74271ccf9ecfa6d780aac0 | |
| #111028 | https://projects.blender.org/blender/blender-addons/commit/3f1674ef5213d1c369f5039614d62742fe4b4a5f | was a duplicate of https://projects.blender.org/blender/blender-addons/issues/104723 [which is in 3.6.3] |
| | 2549272f02bd732260a978e0f98067d1c9a9fb95 | conflicts, assume this needs additional prior commit(s)? |
| #110204 | c275f4219f1482c0a89b0f91179058d60c007643 | conflicts because of 4f3e2ee857b5b9d5625513c3e5258eb71a2314c8, might not be backported |
| #111779 | 30e3caaf8291811f9c86a5721a67a976d18e5af2 | conflicts because of 1b4b90f5f79e3f3b30ba489dec9d35a5c72a58c2, might not be backported, would also need e924f316a7a8c5defcebd417166ebe93dbc8f72a |

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 24, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 24, 2024
@brechtvl
Copy link
Contributor Author

@silverwind: In #31091 the entire .markup was set to overflow-wrap: anywhere. This changes it back to overflow-wrap: break-word for just tables. But I'm not sure if it's that's the right level.

@silverwind
Copy link
Member

silverwind commented Jun 24, 2024

Ah, I did notice as well that table columns were not rendering as nicely in 1.22, this is likely the cause. I will verify this change, I assume it's okay because tables disregard many CSS behaviours that work on other elements, so I assume they ignore/mishandle overflow-wrap which usually works well on non-table elements.

@silverwind
Copy link
Member

silverwind commented Jun 24, 2024

Need to review how GitHub does this to stay compatible with their rendering. I think it might even be an option to have both CSS properties on the .markup root as they do different things and the overflow-wrap was definitely needed to prevent markup content from breaking outside of these code comment boxes.

@silverwind silverwind added the backport/v1.22 This PR should be backported to Gitea 1.22 label Jun 24, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 24, 2024
@silverwind
Copy link
Member

silverwind commented Jun 24, 2024

So I think we should have overflow-wrap: break-word; on .markup to have faithful github-like rendering, but that presents the overflow issue on code comment, which I think should be fixed by targeting this inline code comment box specifically so that it does not break outside its parent:

image

@silverwind
Copy link
Member

Pushed a fix that satisfies both cases, it's not ideal but I find it quite hard to make the text wrap in these conversation-holder:

Without the override:
Screenshot 2024-06-24 at 15 26 08

With it:
Screenshot 2024-06-24 at 15 26 15

Checking for other solutions...

@silverwind
Copy link
Member

I see no easy solutions, so I suggest we find a cleaner fix later and this will suffice for now.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 24, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Jun 24, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 24, 2024
@techknowlogick techknowlogick enabled auto-merge (squash) June 24, 2024 17:43
@techknowlogick techknowlogick added type/bug topic/ui Change the appearance of the Gitea UI reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Jun 24, 2024
@techknowlogick techknowlogick merged commit 053e582 into go-gitea:main Jun 24, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Jun 24, 2024
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Jun 24, 2024
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Jun 24, 2024
wxiaoguang pushed a commit that referenced this pull request Jun 25, 2024
Backport #31473 by brechtvl

Co-authored-by: Brecht Van Lommel <[email protected]>
Co-authored-by: silverwind <[email protected]>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 25, 2024
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Fix poor table column width due to breaking words (go-gitea#31473)
  bump golang deps (go-gitea#31422)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/done All backports for this PR have been created backport/v1.22 This PR should be backported to Gitea 1.22 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants