Skip to content

Highlight selected file in the PR file tree #23947

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
Merged
merged 2 commits into from
Apr 7, 2023
Merged

Highlight selected file in the PR file tree #23947

merged 2 commits into from
Apr 7, 2023

Conversation

yusifeng
Copy link
Contributor

@yusifeng yusifeng commented Apr 6, 2023 •
edited
Loading

before
before

after
after

@sillyguodong sillyguodong added the topic/ui Change the appearance of the Gitea UI label Apr 6, 2023
@KN4CK3R
Copy link
Member

KN4CK3R commented Apr 6, 2023

Could you add before/after screenshots or explain what it does?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 6, 2023
@yusifeng
Copy link
Contributor Author

yusifeng commented Apr 6, 2023

Could you add before/after screenshots or explain what it does?

Could you add before/after screenshots or explain what it does?
I have already add 2 gif in comment

@delvh delvh changed the title Highlighted selection of diff files Highlight selected file in the PR file tree Apr 6, 2023
@delvh delvh added the type/enhancement An improvement of existing functionality label Apr 6, 2023
@delvh delvh added this to the 1.20.0 milestone Apr 6, 2023
@silverwind
Copy link
Member

This is nice, it's something I wanted to add as well.

delvh
delvh reviewed Apr 6, 2023
View reviewed changes
@wxiaoguang
Copy link
Contributor

It seems that the "current selected item" is a global state, maybe we should consider about using https://vuex.vuejs.org/

However, the simplicity quickly breaks down when we have multiple components that share a common state:

@yusifeng yusifeng force-pushed the highlight-selected-diff-file branch from 9084bf9 to 8940d22 Compare April 6, 2023 10:24
@silverwind
Copy link
Member

It seems that the "current selected item" is a global state

You mean shared between the two components? Yes I guess some state management across components would be nice, but I have no experience with vue global state. In React, I use zustand, so something similarily simple would be great.

@yusifeng
Copy link
Contributor Author

yusifeng commented Apr 6, 2023

This is nice, it's something I wanted to add as well.

Compared to other similar tools（like bitbucket）, Gitea render too many file in this page, especially when there are many files changed in a pull request, it may cause the page to lag.

I am prepare to render one file only other files will auto close , like Collapse Compoent.
20230406184013_rec_

@lunny
Copy link
Member

lunny commented Apr 6, 2023

This is nice, it's something I wanted to add as well.

Compared to other similar tools（like bitbucket）, Gitea render too many file in this page, especially when there are many files changed in a pull request, it may cause the page to lag.

I am prepare to render one file only other files will auto close , like Collapse Compoent. 20230406184013_rec_ 20230406184013_rec_

Please do that in another PR, one PR do one thing.

@silverwind
Copy link
Member

Compared to other similar tools（like bitbucket）, Gitea render too many file in this page, especially when there are many files changed in a pull request, it may cause the page to lag.

We have an open issue around for Diff performance with many files. Yes, it's for another PR, not this one.

@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 Apr 6, 2023
silverwind
silverwind requested changes Apr 6, 2023
View reviewed changes
Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Please use --color-active for .selected and keep --color-hover for :hover.

@yusifeng yusifeng force-pushed the highlight-selected-diff-file branch from 8940d22 to 9981146 Compare April 7, 2023 02:24
@yusifeng
Copy link
Contributor Author

yusifeng commented Apr 7, 2023

done!

@codecov-commenter

This comment was marked as off-topic.

Sign in to view
@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 Apr 7, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 7, 2023
@jolheiser jolheiser enabled auto-merge (squash) April 7, 2023 17:08
@jolheiser
Copy link
Member

🎺 🤖

@jolheiser jolheiser merged commit 395c716 into go-gitea:main Apr 7, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 7, 2023
@wxiaoguang wxiaoguang mentioned this pull request Apr 8, 2023
Merged
silverwind added a commit that referenced this pull request Apr 12, 2023
@wxiaoguang @silverwind
Use reactive store to share data between components (#23996)
6892e2b 
Follow #23947

* Use reactive store to share data between components
* Remove no-op `this.hashListener = window.addEventListener()` because
`addEventListener` returns void/undefined.

Reference:


https://vuejs.org/guide/scaling-up/state-management.html#simple-state-management-with-reactivity-api


Screenshot (the same as before):

<details>

<img width="565" alt="image"
src="https://user-images.githubusercontent.com/2114189/230701282-bd61bfa3-7786-433b-9ad8-a88591112a02.png">


</details>

---------

Co-authored-by: silverwind <[email protected]>
@sillyguodong sillyguodong added outdated/backport/v1.19 This PR should be backported to Gitea 1.19 backport/manual No power to the bots! Create your backport yourself! labels Apr 14, 2023
sillyguodong pushed a commit to sillyguodong/gitea that referenced this pull request Apr 14, 2023
@yusifeng @sillyguodong
Highlight selected file in the PR file tree (go-gitea#23947)
270ea2a 
before

![before](https://user-images.githubusercontent.com/36984894/230327904-6e712ca2-f777-4cad-99f3-53bc20008180.gif)

after

![after](https://user-images.githubusercontent.com/36984894/230327966-6e5dd971-f0df-427a-a80b-6a9b6db6065d.gif)
# Conflicts:
#	web_src/js/components/DiffFileTreeItem.vue
@sillyguodong sillyguodong mentioned this pull request Apr 14, 2023
Merged
lunny pushed a commit that referenced this pull request Apr 14, 2023
@sillyguodong @yusifeng
Highlight selected file in the PR file tree (#23947) (#24126)
10f9399 
backport #23947 by @yusifeng 

before

![before](https://user-images.githubusercontent.com/36984894/230327904-6e712ca2-f777-4cad-99f3-53bc20008180.gif)

after

![after](https://user-images.githubusercontent.com/36984894/230327966-6e5dd971-f0df-427a-a80b-6a9b6db6065d.gif)

Co-authored-by: yusifeng <[email protected]>
@lunny lunny added the backport/done All backports for this PR have been created label Jul 26, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
@GiteaBot
Copy link
Collaborator

GiteaBot commented Aug 1, 2023

We lock pull requests 3 months after they were closed. If there's any need for further discussion, please open a new issue. 🍵

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@silverwind silverwind silverwind 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/manual No power to the bots! Create your backport yourself! lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.
10 participants
@yusifeng @KN4CK3R @silverwind @wxiaoguang @lunny @codecov-commenter @jolheiser @GiteaBot @delvh @sillyguodong