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

Refactor branch/tag selector dropdown (first step) #23394

Merged
merged 7 commits into from
Mar 11, 2023

Conversation

wxiaoguang
Copy link
Contributor

Follow:

The branch/tag selector dropdown mixes jQuery/Fomantic UI/Vue together, it's very diffcult to maintain and causes unfixable a11y problems. It also causes problems like #19851 #21314 #21952

This PR is the first step for the refactoring, move data- attributes to JS object and use Vue data as much as possible.

The old selector '.choose.reference .dropdown' was also wrong, it hits <div class="choose reference"><svg class="dropdown icon"> and would cause undefined behaviors.

I have done some quick tests and it works. After this PR gets merged, I will move the code into a Vue SFC in next PR.

image

image

@wxiaoguang wxiaoguang force-pushed the refactor-branch-selector branch from f376fd8 to f178c00 Compare March 9, 2023 17:05
@silverwind
Copy link
Member

Thanks for untangling this abomination.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 9, 2023
@silverwind
Copy link
Member

Does it work for all three cases (branch, tag, commit)?

@wxiaoguang
Copy link
Contributor Author

Does it work for all three cases (branch, tag, commit)?

I think yes, feel free to have a try.

@wxiaoguang wxiaoguang force-pushed the refactor-branch-selector branch from e9c2ac6 to 703dfd3 Compare March 9, 2023 20:26
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.

Looks to work.

@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 Mar 9, 2023
@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Mar 10, 2023
@lunny lunny added this to the 1.20.0 milestone Mar 10, 2023
@wxiaoguang wxiaoguang force-pushed the refactor-branch-selector branch from 99b961d to 09422ee Compare March 10, 2023 07:24
@wxiaoguang wxiaoguang force-pushed the refactor-branch-selector branch from 09422ee to b05907e Compare March 10, 2023 07:27
@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 Mar 10, 2023
@lafriks lafriks merged commit 75022f8 into go-gitea:main Mar 11, 2023
@wxiaoguang wxiaoguang deleted the refactor-branch-selector branch March 11, 2023 10:47
@wxiaoguang
Copy link
Contributor Author

Next: Refactor branch/tag selector to Vue SFC #23421

@delvh delvh added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 12, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 13, 2023
* giteaofficial/main:
  Scoped label display and documentation tweaks (go-gitea#23430)
  Deduplicate template code for label selection menu (go-gitea#23431)
  Show edit/close/delete button on organization wide repositories (go-gitea#23388)
  Sync the class change of Edit Column Button to JS code (go-gitea#23400)
  Preserve file size when creating attachments (go-gitea#23406)
  [skip ci] Updated translations via Crowdin
  Use buildkit for docker builds (go-gitea#23415)
  Refactor branch/tag selector dropdown (first step) (go-gitea#23394)
  [skip ci] Updated translations via Crowdin
  Hide target selector if tag exists when creating new release (go-gitea#23171)
  Parse external request id from request headers, and print it in access log (go-gitea#22906)
  Add missing tabs to org projects page (go-gitea#22705)
  Add user webhooks (go-gitea#21563)
  Handle OpenID discovery URL errors a little nicer when creating/editing sources (go-gitea#23397)
  Split CI pipelines (go-gitea#23385)
lunny pushed a commit that referenced this pull request Mar 14, 2023
Similar to #23394

The dashboard repo list mixes jQuery/Fomantic UI/Vue together, it's very
diffcult to maintain and causes unfixable a11y problems.

This PR uses two steps to refactor the repo list:

1. move `data-` attributes to JS object and use Vue data as much as
possible
d3adc0d
2. move the code into a Vue SFC
7ebe55d

Total: +516 −585

Screenshots:

<details>

![image](https://user-images.githubusercontent.com/2114189/224271457-a23e05be-d7d3-4247-a803-f0ee30c36f44.png)

![image](https://user-images.githubusercontent.com/2114189/224271504-76fbd3da-4d7a-4725-b0d1-fbff83caac63.png)

![image](https://user-images.githubusercontent.com/2114189/224271845-f007cadf-6c49-46bd-a65c-a3fc75bdba3b.png)

</details>

---------

Co-authored-by: John Olheiser <john.olheiser@gmail.com>
lunny added a commit that referenced this pull request Mar 14, 2023
Follow #23394

There were many bad smells in old code. This PR only moves the code into
Vue SFC, doesn't touch the unrelated logic.

update: after
5f23218
, there should be no usage of the vue-rumtime-compiler anymore
(hopefully), so I think this PR could close #19851

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants