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.

Sign up for GitHub

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

Jump to bottom

Replace ajax with fetch, improve image diff #27267

Merged
merged 29 commits into from
Oct 11, 2023
Merged

Replace ajax with fetch, improve image diff #27267

merged 29 commits into from
Oct 11, 2023

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Sep 25, 2023 •
edited
Loading

  1. Dropzone attachment removal, pretty simple replacement
  2. Image diff: The previous code fetched every image twice, once via img[src] and once via $.ajax. Now it's only fetched once and a second time only when necessary. The image diff code was partially rewritten.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 25, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 25, 2023
@github-actions github-actions bot added the topic/ui Change the appearance of the Gitea UI label Sep 25, 2023
@silverwind
Copy link
Member Author

silverwind commented Sep 25, 2023 •
edited
Loading

Likely should move the DOMParser and XMLSerializer to some shared functions.

Edit: done.

@silverwind silverwind marked this pull request as draft September 25, 2023 22:05
@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 Sep 25, 2023
@silverwind silverwind marked this pull request as ready for review September 25, 2023 22:22
@silverwind
Copy link
Member Author

I guess we can backport as an enhancement to image diff which should now load twice as fast and use half the bandwidth.

@silverwind silverwind added the backport/v1.21 This PR should be backported to Gitea 1.21 label Sep 25, 2023
wxiaoguang
wxiaoguang previously requested changes Sep 25, 2023
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seem not right at the moment.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 25, 2023
wxiaoguang
wxiaoguang reviewed Sep 26, 2023
View reviewed changes
@silverwind silverwind marked this pull request as draft September 26, 2023 15:01
@silverwind silverwind marked this pull request as ready for review September 29, 2023 18:26
@silverwind
Copy link
Member Author

Ready again.

@silverwind silverwind added type/refactoring Existing code has been cleaned up. There should be no new functionality. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Sep 29, 2023
wxiaoguang
wxiaoguang reviewed Sep 30, 2023
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to wrap some await with try-catch.

Because now we show the unhandled rejected promise error to end users. But for this case (get SVG size), it doesn't need to show the errors.

@wxiaoguang wxiaoguang self-requested a review September 30, 2023 02:04
@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 Oct 11, 2023
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 11, 2023
@silverwind silverwind enabled auto-merge (squash) October 11, 2023 11:31
@silverwind silverwind disabled auto-merge October 11, 2023 11:32
@silverwind silverwind removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 11, 2023
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 11, 2023
@silverwind silverwind enabled auto-merge (squash) October 11, 2023 11:36
@silverwind silverwind merged commit 73b63d9 into go-gitea:main Oct 11, 2023
@GiteaBot GiteaBot added this to the 1.22.0 milestone Oct 11, 2023
GiteaBot added a commit to GiteaBot/gitea that referenced this pull request Oct 11, 2023
@silverwind @GiteaBot
Replace ajax with fetch, improve image diff (go-gitea#27267)
703a2aa 
1. Dropzone attachment removal, pretty simple replacement
2. Image diff: The previous code fetched every image twice, once via
`img[src]` and once via `$.ajax`. Now it's only fetched once and a
second time only when necessary. The image diff code was partially
rewritten.

---------

Co-authored-by: Giteabot <teabot@gitea.io>
@GiteaBot GiteaBot mentioned this pull request Oct 11, 2023
Merged
@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 Oct 11, 2023
@silverwind silverwind deleted the fetch1 branch October 11, 2023 12:49
silverwind added a commit that referenced this pull request Oct 11, 2023
@GiteaBot @silverwind
Replace ajax with fetch, improve image diff (#27267) (#27583)
7ec7c73 
Backport #27267 by @silverwind

1. Dropzone attachment removal, pretty simple replacement
2. Image diff: The previous code fetched every image twice, once via
`img[src]` and once via `$.ajax`. Now it's only fetched once and a
second time only when necessary. The image diff code was partially
rewritten.

Co-authored-by: silverwind <me@silverwind.io>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 12, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
b80e30c 
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Keep filter when showing unfiltered results on explore page (go-gitea#27192)
  Don't show Link to TOTP if not set up (go-gitea#27585)
  Fix data-race bug when accessing task.LastRun (go-gitea#27584)
  Fix template bug (go-gitea#27581)
  Replace ajax with fetch, improve image diff (go-gitea#27267)
  Replace assert.Fail with assert.FailNow (go-gitea#27578)
  Fix the robots.txt path
  show manual cron run's last time (go-gitea#27544)
  fully replace drone with actions (go-gitea#27556)
  Revert "Simplify `contrib/backport` (go-gitea#27520)" (go-gitea#27566)
  Align ISSUE_TEMPLATE with the new label system (go-gitea#27573)
  Penultimate round of `db.DefaultContext` refactor (go-gitea#27414)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@puni9869 puni9869 puni9869 left review comments

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/ui Change the appearance of the Gitea UI type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.
5 participants
@silverwind @puni9869 @wxiaoguang @delvh @GiteaBot