Skip to content

Highlight viewed files differently in the PR filetree#24956

Merged
lunny merged 5 commits into
go-gitea:mainfrom
sebastian-sauer:feature/24566_show_file_viewed_status_in_tree
Jun 25, 2023
Merged

Highlight viewed files differently in the PR filetree#24956
lunny merged 5 commits into
go-gitea:mainfrom
sebastian-sauer:feature/24566_show_file_viewed_status_in_tree

Conversation

@sebastian-sauer

@sebastian-sauer sebastian-sauer commented May 27, 2023

Copy link
Copy Markdown
Contributor

image

fixes #24566

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 27, 2023
sebastian-sauer
sebastian-sauer commented May 27, 2023
View reviewed changes
Comment thread web_src/js/features/pull-view-file.js Outdated
@delvh delvh changed the title Show IsViewed Status in filetree Highlight viewed files differently in the PR filetree May 27, 2023
delvh
delvh approved these changes May 27, 2023
View reviewed changes
Comment thread web_src/js/features/pull-view-file.js Outdated
@delvh delvh added type/enhancement An improvement of existing functionality topic/ui Change the appearance of the Gitea UI labels May 27, 2023
@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 May 28, 2023
@lunny

lunny commented May 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Maybe the files should have a blue color to keep consistent with the checkbox color on the right column.

@silverwind

silverwind commented May 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Maybe the files should have a blue color to keep consistent with the checkbox color on the right column.

I would add a checkmark octicon in text color on the right side of the file name instead.

Maybe we need to also expand the width of the sidebar by 16px or more to accomodate this additional icon.

@delvh

delvh commented May 29, 2023

Copy link
Copy Markdown
Member

@silverwind see also #24566 (comment) …

@silverwind

silverwind commented May 29, 2023

Copy link
Copy Markdown
Member

The file icon is not completely useless because symlink should show differently. But yes, we can use it for this checkmark icon too, to conserve space.

@sebastian-sauer sebastian-sauer mentioned this pull request May 29, 2023
Closed
@pull-request-size pull-request-size Bot added size/L and removed size/S labels May 29, 2023
silverwind
silverwind reviewed May 29, 2023
View reviewed changes
Comment thread web_src/js/features/repo-issue.js Outdated
silverwind
silverwind reviewed May 29, 2023
View reviewed changes
Comment thread templates/repo/diff/box.tmpl Outdated
silverwind
silverwind reviewed May 29, 2023
View reviewed changes
Comment thread templates/repo/diff/box.tmpl Outdated
silverwind
silverwind reviewed May 29, 2023
View reviewed changes
Comment thread web_src/js/modules/stores.js Outdated

@silverwind silverwind May 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can files be made a Set? There can never be duplicates in the file paths. Also I think files is not a good name, maybe viewedFiles, assuming it's there only to tracked viewed files?

@sebastian-sauer sebastian-sauer May 29, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not only a tracker of the viewed files - it's the list of all files shown in the diff.

@silverwind silverwind May 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, still could be made a Set then, to make existance checks O(1) instead of O(n).

wxiaoguang
wxiaoguang reviewed May 30, 2023
View reviewed changes
Comment thread templates/repo/diff/box.tmpl Outdated
@wxiaoguang wxiaoguang mentioned this pull request May 30, 2023
Merged
lunny pushed a commit that referenced this pull request May 30, 2023
@wxiaoguang
Refactor diffFileInfo / DiffTreeStore (#24998)
ee99cf6 
Follow  #21012, #22399

Replace #24983, fix #24938

Help #24956

Now, the `window.config.pageData.diffFileInfo` itself is a reactive
store, so it's quite easy to sync values/states by it, no need to do
"doLoadMoreFiles" or "callback".

Screenshot: these two buttons both work. After complete loading, the UI
is also right.

<details>


![image](https://github.com/go-gitea/gitea/assets/2114189/cc6310fd-7f27-45ea-ab4f-24952a87b421)


![image](https://github.com/go-gitea/gitea/assets/2114189/4c11dd67-ac03-4568-8541-91204d27a4e3)


![image](https://github.com/go-gitea/gitea/assets/2114189/38a22cec-41be-41e6-a209-f347b7a4c1de)

</details>
@silverwind

silverwind commented May 30, 2023

Copy link
Copy Markdown
Member

Now that #24998 is in, this needs a few adoptions.

@sebastian-sauer sebastian-sauer force-pushed the feature/24566_show_file_viewed_status_in_tree branch from 3d79f82 to d299564 Compare May 31, 2023 16:01
@pull-request-size pull-request-size Bot added size/S and removed size/L labels May 31, 2023
wxiaoguang
wxiaoguang reviewed Jun 5, 2023
View reviewed changes
Comment thread web_src/js/features/pull-view-file.js Outdated
Comment thread web_src/js/components/DiffFileTreeItem.vue Outdated
Comment thread web_src/js/components/DiffFileTreeItem.vue Outdated
@wxiaoguang

wxiaoguang commented Jun 24, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Ping, does it get stale?

@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Jun 24, 2023
@sebastian-sauer

sebastian-sauer commented Jun 24, 2023

Copy link
Copy Markdown
Contributor Author

Ping, does it get stale?

Sorry - will add the changes and test it now.

sebastian-sauer and others added 2 commits June 24, 2023 17:07
@sebastian-sauer
Merge remote-tracking branch 'origin/main' into feature/24566_show_fi…
4bb5e52 
…le_viewed_status_in_tree
@sebastian-sauer @wxiaoguang
Apply suggestions from code review
4727df4 
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@wxiaoguang wxiaoguang removed the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Jun 24, 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 24, 2023
@sebastian-sauer

sebastian-sauer commented Jun 24, 2023

Copy link
Copy Markdown
Contributor Author

All changes applied - ready to review.

@wxiaoguang wxiaoguang added this to the 1.21.0 milestone Jun 24, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 25, 2023
@lunny

lunny commented Jun 25, 2023

Copy link
Copy Markdown
Member

I think there is better way to indicate the viewed files on the left tree but let's merge this one and expect another PR to do that.

@lunny lunny merged commit 77e449f into go-gitea:main Jun 25, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 25, 2023
@delvh delvh mentioned this pull request Jul 3, 2023
Closed
@sebastian-sauer sebastian-sauer deleted the feature/24566_show_file_viewed_status_in_tree branch July 9, 2023 19:36
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@silverwind silverwind silverwind left review comments

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@delvh delvh delvh approved these changes

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality

Projects

None yet

Milestone

1.21.0

Development

Successfully merging this pull request may close these issues.

There are some difference between reviewed and unreviewed files on the left changed files tree.

6 participants

@sebastian-sauer @lunny @silverwind @delvh @wxiaoguang @GiteaBot