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

Refactor repo clone button and repo clone links, fix JS error on empty repo page #19208

Merged
merged 7 commits into from
Mar 29, 2022
Merged

Refactor repo clone button and repo clone links, fix JS error on empty repo page #19208

merged 7 commits into from
Mar 29, 2022

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Mar 25, 2022 •
edited
Loading

The last PR about clone buttons introduced an JS error when visiting an empty repo page:

This PR:

  1. Simplify templates/repo/clone_buttons.tmpl and make code clear
  2. Move most JS code into initRepoCloneLink
  3. Remove unused CloneLink.Git
  4. Remove ctx.Data["DisableSSH"] / ctx.Data["ExposeAnonSSH"] / ctx.Data["DisableHTTP"], and only set them when is is needed (eg: deploy keys / ssh keys)
  5. Introduce Data["CloneButton*"] to provide data for clone buttons and links
  6. Introduce Data["RepoCloneLink"] for the repo clone link (not the wiki)
  7. Remove most ctx.Data["PageIsWiki"] because it has been set in the /wiki middleware
  8. Remove incorrect quickstart class in migrating.tmpl

The backport fix for 1.16 is #19209, which is a very simple fix, no refactoring.

Screenshot

Disable SSH / Disable SSH and HTTP

image

Disable HTTP

image

Both HTTP and SSH

image

Wiki Page

image

go-get

image

@wxiaoguang wxiaoguang requested a review from zeripath March 25, 2022 08:26
@wxiaoguang wxiaoguang added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 25, 2022
@wxiaoguang wxiaoguang mentioned this pull request Mar 25, 2022
Closed
@wxiaoguang wxiaoguang force-pushed the refactor-clone-button branch from c93d979 to 6cff34c Compare March 25, 2022 08:32
@wxiaoguang wxiaoguang added type/bug backport/done All backports for this PR have been created labels Mar 25, 2022
@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Mar 25, 2022
@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Mar 25, 2022
@wxiaoguang wxiaoguang force-pushed the refactor-clone-button branch from 6cff34c to 0632a73 Compare March 25, 2022 09:06
@wxiaoguang wxiaoguang force-pushed the refactor-clone-button branch from 0632a73 to f2ef337 Compare March 25, 2022 09:12
zeripath
zeripath reviewed Mar 26, 2022
View reviewed changes
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

Few minor English changes.

Otherwise I thought we were going to try to remove jQuery in an opportunistic manner - this seems like an opportune refactor.

templates/repo/clone_buttons.tmpl Outdated Show resolved Hide resolved
modules/context/repo.go Outdated Show resolved Hide resolved
templates/repo/clone_buttons.tmpl Outdated Show resolved Hide resolved
templates/repo/clone_buttons.tmpl Outdated Show resolved Hide resolved
templates/repo/empty.tmpl Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 26, 2022
@wxiaoguang @zeripath
Apply suggestions from code review
38e99b0 
Co-authored-by: zeripath <art27@cantab.net>
@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 26, 2022 •
edited
Loading

Few minor English changes.

Updated

Otherwise I thought we were going to try to remove jQuery in an opportunistic manner - this seems like an opportune refactor.

  1. I didn't see the conclusion and plans from Frontend discussion: jQuery & Fomantic-UI #18302 about how to drop jQuery.
  2. I don't think we can drop jQuery if we are still using go-tmpl + Fomantic UI, we should stop from mixing native + jQuery + Fomantic UI, togehter, it would be a nightmare.
  3. I do not think there is any downside about using jQuery (for tmpl, correctly), neither performance, nor readability.
  4. These code(initRepoClone) before was using jQuery, and it had to be admitted that the code with jQuery is much easier to be maintained (short and safe) than native.

@wxiaoguang wxiaoguang changed the title Refactor repo clone button and repo clone links Refactor repo clone button and repo clone links, fix JS error on empty repo page Mar 26, 2022
@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 27, 2022
@wxiaoguang wxiaoguang requested a review from 6543 March 28, 2022 01:50
@wxiaoguang wxiaoguang requested a review from lunny March 28, 2022 16:51
@6543 6543 mentioned this pull request Mar 29, 2022
Merged
@GiteaBot GiteaBot removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 29, 2022
@GiteaBot GiteaBot added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Mar 29, 2022
@6543 6543 merged commit d4c789d into go-gitea:main Mar 29, 2022
@wxiaoguang wxiaoguang deleted the refactor-clone-button branch March 29, 2022 03:21
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 30, 2022
@zjjhot
Merge remote-tracking branch 'giteaoffical/main'
f6c0470 
* giteaoffical/main: (31 commits)
  Add Package Registry (go-gitea#16510)
  Show messages for users if the ROOT_URL is wrong, show JavaScript errors (go-gitea#18971)
  [skip ci] Updated translations via Crowdin
  Make git.OpenRepository accept Context (go-gitea#19260)
  Use full output of git show-ref --tags to get tags for PushUpdateAddTag (go-gitea#19235)
  When conflicts have been previously detected ensure that they can be resolved (go-gitea#19247)
  More commit info from API (go-gitea#19252)
  Move some issue methods as functions (go-gitea#19255)
  Move project files into models/project sub package (go-gitea#17704)
  Granular webhook events in editHook (go-gitea#19251)
  Provide configuration to allow camo-media proxying (go-gitea#12802)
  Move init repository related functions to modules (go-gitea#19159)
  Move organization related structs into sub package (go-gitea#18518)
  Refactor repo clone button and repo clone links, fix JS error on empty repo page (go-gitea#19208)
  Show last cron messages on monitor page (go-gitea#19223)
  Allow API to create file on empty repo (go-gitea#19224)
  Use goproxy.io instead of goproxy.cn (go-gitea#19242)
  New cron task: delete old system notices (go-gitea#19219)
  Let web and API routes have different auth methods group (go-gitea#19168)
  Only send webhook events to active system webhooks and only deliver to active hooks (go-gitea#19234)
  ...
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@6543 6543 6543 approved these changes

@zeripath zeripath zeripath approved these changes

@lunny lunny Awaiting requested review from lunny

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.
4 participants
@wxiaoguang @zeripath @6543 @GiteaBot