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 loading state regression in markup content #25349

Merged
merged 7 commits into from
Jun 19, 2023
Merged

Conversation

silverwind
Copy link
Member

Fix regressions from #25219:

Math before and after:
Screenshot 2023-06-18 at 16 00 52
Screenshot 2023-06-18 at 16 03 44

Mermain before and after:
Screenshot 2023-06-18 at 15 58 56
Screenshot 2023-06-18 at 15 58 37

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 18, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 18, 2023
@silverwind
Copy link
Member Author

@wxiaoguang There is one regression here by this change:

image

But I can't keep this max-height as it distorts this spinner:

Screenshot 2023-06-18 at 16 04 48

Any idea how to fix both? aspect-ratio seems to do nothing.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jun 18, 2023

Any idea how to fix both? aspect-ratio seems to do nothing.

I guess there is a "width" style for it? "width" overrides "aspect-ratio" Could you help to provide some reproducible code

@silverwind
Copy link
Member Author

Seems setting both max-width and max-height seems to have fixed it:

Screenshot 2023-06-18 at 16 13 01 Screenshot 2023-06-18 at 16 14 18

@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 18, 2023
@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 19, 2023
@lunny lunny added issue/regression Issue needs no code to be fixed, only a description on how to fix it yourself skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Jun 19, 2023
@lunny lunny enabled auto-merge (squash) June 19, 2023 04:52
@lunny
Copy link
Member

lunny commented Jun 19, 2023

workflow maybe wrong, test-e2e hasn't been triggered.

@silverwind
Copy link
Member Author

This PR should only trigger frontend pipelines.

@silverwind
Copy link
Member Author

e2e is running, looks fine now.

@lunny lunny merged commit c09d0b4 into go-gitea:main Jun 19, 2023
@GiteaBot GiteaBot added this to the 1.21.0 milestone Jun 19, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 19, 2023
@silverwind silverwind deleted the load branch June 19, 2023 08:23
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 19, 2023
* giteaofficial/main:
  Fix incorrect actions ref_name (go-gitea#25358)
  Make backend code respond correct JSON when creating PR (go-gitea#25353)
  Fix loading state regression in markup content (go-gitea#25349)
  Batch delete issue and improve tippy opts (go-gitea#25253)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/regression Issue needs no code to be fixed, only a description on how to fix it yourself lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants