Prevent invalid behavior for file reviewing when loading more files #21230

delvh · 2022-09-21T11:43:19Z

The problem was that many PR review components loaded by Show more received the same ID as previous batches, which confuses browsers (when clicked).
All such occurrences should now be fixed.

Additionally improved the background of the viewed checkbox:

Lastly, the go-licenses.json was automatically updated.

Fixes #21228.
Fixes #20681.

The problem was that later loaded files got the same ID, which confused the browsers when clicked. Additionally auto-update the go-licenses.json

templates/repo/diff/box.tmpl

delvh · 2022-09-21T11:46:37Z

I've noticed that this problem is present at least once more in this block (there are multiple other occurrences of {{i}} in the same range), however, I don't know where this might all be called, so I left it untouched for now.
If someone knows where the other ones are called, please feel free to fix it either in this PR or a new one.

wxiaoguang · 2022-09-21T11:57:11Z

I've noticed that this problem is present at least once more in this block (there are multiple other occurrences of {{i}} in the same range), however, I don't know where this might all be called, so I left it untouched for now. If someone knows where the other ones are called, please feel free to fix it either in this PR or a new one.

Good catch, I think these {{$i}} should be fixed together. If you do not mind, I can submit my changes on this PR.

delvh · 2022-09-21T12:02:11Z

I have nothing against that.

wxiaoguang · 2022-09-21T12:09:38Z

One more thing .... should we remove the background of the checked viewed input?

It looks a little strange ....

Or maybe this (like GitHub)

delvh · 2022-09-21T12:14:09Z

I don't think it's a good idea to remove it completely as it definitely helps with skimming through a PR,
but if you want to enhance it, I'm all for it.
I'm not really a designer, so better designs are always welcome.

templates/repo/diff/box.tmpl

delvh · 2022-09-21T12:15:45Z

Ah, the other changes didn't require any JS changes at all?

wxiaoguang · 2022-09-21T12:17:03Z

Ah, the other changes didn't require any JS changes at all?

Yes, if I understand correctly, the other changes are only for SVG/CSV diff, to toggle the source. These IDs are used by data-toggle-selector, luckily they are on the same page.

wxiaoguang · 2022-09-21T12:37:33Z

I don't think it's a good idea to remove it completely as it definitely helps with skimming through a PR, but if you want to enhance it, I'm all for it. I'm not really a designer, so better designs are always welcome.

How do you think about this? (I could propose a new PR if we want to keep this PR simple)

delvh · 2022-09-21T13:12:01Z

Fine with me.
I have nothing against adding it to this PR.

silverwind · 2022-09-21T13:44:44Z

Lastly, the go-licenses.json was automatically updated.

Odd, must be a bug in go-licenses, our go dependencies did not change since last generation.

wxiaoguang · 2022-09-21T13:48:56Z

Lastly, the go-licenses.json was automatically updated.

Odd, must be a bug in go-licenses, our go dependencies did not change since last generation.

I also experience such problem. I just ignore(reset) that file everytime. It's good to get it in.

delvh · 2022-09-21T13:50:20Z

Well, it's inside our go.mod (

gitea/go.mod

Line 106 in 0a9a86b

gopkg.in/yaml.v3 v3.0.1

), and it wasn't inside go-licenses.json previously…

silverwind · 2022-09-21T15:18:00Z

Well, it's inside our go.mod (

gitea/go.mod

Line 106 in 0a9a86b

gopkg.in/yaml.v3 v3.0.1

), and it wasn't inside go-licenses.json previously…

Might be because gopkg.in is down sometimes, so the license fetcher can not fetch license from there. We should probably exclude licenses from there if the service is unstable.

Edit: Thought, there are more packages from gopkg.in that also appear to be unaffected, so maybe it's a wrong lead.

Also use of a GOPROXY should prevent such issues in first place. Maybe we should just recommend setting one for all developers.

…o-gitea#21230) The problem was that many PR review components loaded by `Show more` received the same ID as previous batches, which confuses browsers (when clicked). All such occurrences should now be fixed. Additionally improved the background of the `viewed` checkbox. Lastly, the `go-licenses.json` was automatically updated. Fixes go-gitea#21228. Fixes go-gitea#20681. Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

* upstream/main: Use absolute links in feeds (go-gitea#21229) Prevent invalid behavior for file reviewing when loading more files (go-gitea#21230) Respect `REQUIRE_SIGNIN_VIEW` for packages (go-gitea#20873) Make Clone in VSCode link get updated correctly (go-gitea#21225) Configure golangci-lint to show all issues (go-gitea#21106) Fix user visible check (go-gitea#21210) Fix template bug of admin monitor (go-gitea#21208) Clarify that `ENABLE_SWAGGER` only influences the API docs, not the routes (go-gitea#21215) Enable fluid page layout on medium size viewports (go-gitea#21178) [API] teamSearch show teams with no members if user is admin (go-gitea#21204)

…21230) (#21234) Backport of #21230 The problem was that many PR review components loaded by `Show more` received the same ID as previous batches, which confuses browsers (when clicked). All such occurrences should now be fixed. Additionally improved the background of the `viewed` checkbox. Fixes #21228. Fixes #20681. Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

Prevent invalid behavior for file reviewing when loading more files

2736089

The problem was that later loaded files got the same ID, which confused the browsers when clicked. Additionally auto-update the go-licenses.json

wxiaoguang reviewed Sep 21, 2022

View reviewed changes

templates/repo/diff/box.tmpl Outdated Show resolved Hide resolved

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 21, 2022

Move checkbox into the label

7e91d95

use $file.NameHash instead of unstable $i

e0813e4

wxiaoguang approved these changes Sep 21, 2022

View reviewed changes

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 21, 2022

wxiaoguang reviewed Sep 21, 2022

View reviewed changes

templates/repo/diff/box.tmpl Outdated Show resolved Hide resolved

lint

2424b63

wxiaoguang added type/bug backport/v1.17 labels Sep 21, 2022

wxiaoguang added this to the 1.18.0 milestone Sep 21, 2022

wxiaoguang added 3 commits September 21, 2022 22:00

fine tune the UI

11cb349

lint

e7d56bd

lint

36388bc

KN4CK3R approved these changes Sep 21, 2022

View reviewed changes

GiteaBot removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Sep 21, 2022

GiteaBot added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Sep 21, 2022

wxiaoguang added 2 commits September 22, 2022 00:02

Merge branch 'main' into no-duplicate-viewed-checkbox-ids

ff8d114

fix new go fmt

a04e7ac

wxiaoguang approved these changes Sep 21, 2022

View reviewed changes

wxiaoguang merged commit acee32c into go-gitea:main Sep 21, 2022

delvh deleted the no-duplicate-viewed-checkbox-ids branch September 21, 2022 17:49

delvh mentioned this pull request Sep 21, 2022

Prevent invalid behavior for file reviewing when loading more files (#21230) #21234

Merged

delvh added the backport/done All backports for this PR have been created label Sep 21, 2022

delvh mentioned this pull request Sep 22, 2022

Add API endpoint to get changed files of a PR #21177

Merged

delvh mentioned this pull request Oct 9, 2022

Pin Repositories on user page (Fixes #10375) #19831

Closed

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent invalid behavior for file reviewing when loading more files #21230

Prevent invalid behavior for file reviewing when loading more files #21230

delvh commented Sep 21, 2022 •

edited

Loading

delvh commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022

delvh commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022 •

edited

Loading

delvh commented Sep 21, 2022

delvh commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022 •

edited

Loading

delvh commented Sep 21, 2022 •

edited

Loading

silverwind commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022

delvh commented Sep 21, 2022 •

edited

Loading

silverwind commented Sep 21, 2022 •

edited

Loading

Prevent invalid behavior for file reviewing when loading more files #21230

Prevent invalid behavior for file reviewing when loading more files #21230

Conversation

delvh commented Sep 21, 2022 • edited Loading

delvh commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022

delvh commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022 • edited Loading

delvh commented Sep 21, 2022

delvh commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022 • edited Loading

delvh commented Sep 21, 2022 • edited Loading

silverwind commented Sep 21, 2022

wxiaoguang commented Sep 21, 2022

delvh commented Sep 21, 2022 • edited Loading

silverwind commented Sep 21, 2022 • edited Loading

delvh commented Sep 21, 2022 •

edited

Loading

wxiaoguang commented Sep 21, 2022 •

edited

Loading

wxiaoguang commented Sep 21, 2022 •

edited

Loading

delvh commented Sep 21, 2022 •

edited

Loading

delvh commented Sep 21, 2022 •

edited

Loading

silverwind commented Sep 21, 2022 •

edited

Loading