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

Rearrange Clone Panel #23231

Closed
wants to merge 1 commit into from
Closed

Conversation

BlenderDefender
Copy link
Contributor

Rearrange the clone panel to use less horizontal space. The following changes have been made to achieve this:

  • Moved everything into the dropdown menu
  • Moved the HTTPS/SSH Switch to a separate line and made the buttons larger
  • Added a short description what HTTPS- and SSH-Cloning are for
  • Moved the "Clone in VS Code"-Button up and added a divider to separate cloning and downloading
  • Named the dropdown button "Code", added appropriate icon and made the button a primary button to raise more attention.

Before:
grafik

After:
grafik

This pull requests aims to fix #23202

@jolheiser jolheiser added this to the 1.20.0 milestone Mar 1, 2023
@jolheiser jolheiser added the topic/ui Change the appearance of the Gitea UI label Mar 1, 2023
@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 1, 2023
@silverwind
Copy link
Member

silverwind commented Mar 1, 2023

Keep note that there is a citation button in this menu (#19999) which makes less sense in such a popup. Maybe just remove the or line to remove the expectation that everything in the menu is related to getting the code.

@silverwind
Copy link
Member

I'd prefer this to not use fomantic dropdown but tippy.js instead.

Assign classnames code-btn and code-popup and init via JS like this:

function initCodeButton() {
  const codeBtn = document.querySelector('.code-btn');
  if (!codeBtn) return;

  createTippy(codeBtn, {
    content: document.querySelector('.code-popup'),
    placement: 'bottom-end',
    trigger: 'click',
    maxWidth: 'none',
    interactive: true,
    hideOnClick: true,
  }); 
}

@BlenderDefender
Copy link
Contributor Author

Keep note that there is a citation button in this menu (#19999) which makes less sense in such a popup. Maybe just remove the or line to remove the expectation that everything in the menu is related to getting the code.

Oh, I overlooked this, I'll fix this ASAP

@BlenderDefender
Copy link
Contributor Author

I'd prefer this to not use fomantic dropdown but tippy.js instead.

Assign classnames code-btn and code-popup and init via JS like this:

function initCodeButton() {
  const codeBtn = document.querySelector('.code-btn');
  if (!codeBtn) return;

  createTippy(codeBtn, {
    content: document.querySelector('.code-popup'),
    placement: 'bottom-end',
    trigger: 'click',
    maxWidth: 'none',
    interactive: true,
    hideOnClick: true,
  }); 
}

Ok, thanks for the feedback

@silverwind
Copy link
Member

You can clone https://try.gitea.io/silverwind/symlink-test which has a CITATION.cff that tiggers the display of the citation link.

@silverwind
Copy link
Member

Some critique:

  • I do not like about this is that it takes two clicks to copy a SSH URL in GitHub UI, while this can be done in one click in the Gitea UI.
  • I'm not sure we are really starved for space like GitHub is with their condensed layout. We do have more space available to show more stuff than they do.

@wxiaoguang
Copy link
Contributor

A good news, after refactoring the Clone Panel, all the code in clone_script.tmpl could be moved into a JS file.

@codecov-commenter

This comment was marked as off-topic.

@silverwind
Copy link
Member

silverwind commented Mar 2, 2023

I'd prefer this to not use fomantic dropdown but tippy.js instead.
Assign classnames code-btn and code-popup and init via JS like this:

function initCodeButton() {
  const codeBtn = document.querySelector('.code-btn');
  if (!codeBtn) return;

  createTippy(codeBtn, {
    content: document.querySelector('.code-popup'),
    placement: 'bottom-end',
    trigger: 'click',
    maxWidth: 'none',
    interactive: true,
    hideOnClick: true,
  }); 
}

Ok, thanks for the feedback

Oh, and I forgot to mention you need to add a tippy-target class to the content element to hide it until the button is clicked. That said, we should eventually devise a system that lets one create tippy popups purely from HTML, but that's something for later.

I think we should name the two elements .code-btn and .code-btn-content.

@BlenderDefender
Copy link
Contributor Author

Using tippy breaks the layout, do you know a fix for this?

grafik

@silverwind
Copy link
Member

Hmm, it shouldn't, but if you could rebase this, maybe I'll play with it :)

Note that Less has been removed, so then change in less/_repository.less now needs to be done in css/repository.css .

@wxiaoguang
Copy link
Contributor

Is this PR active?

@wxiaoguang wxiaoguang added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Apr 24, 2023
@BlenderDefender
Copy link
Contributor Author

Is this PR active?

It still should be, but I unfortunately didn't have time to work on it througout the last month. Will try to do on the weekend.

@wxiaoguang
Copy link
Contributor

If there is any problem, feel free to ask, I might do some help because I also hope we can clean the clone_script.tmpl after the refactoring 😁 Thank you very much!

@wxiaoguang wxiaoguang removed the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Apr 24, 2023
@delvh delvh removed this from the 1.20.0 milestone Jun 4, 2023
@a1012112796
Copy link
Member

stale?

@BlenderDefender
Copy link
Contributor Author

@a1012112796 I guess you can mark it as stale for now, I don't have the time to work on this atm, I'll update the files and the pr as soon as I find time for it. (Hopefully soon)

@lunny lunny added the pr/wip This PR is not ready for review label Dec 5, 2023
@BlenderDefender
Copy link
Contributor Author

Okay, so, I'm trying to work on it again, and I wanted to ask, if tippy.js is still the way to go. If yes, what do I have to do to use tippy.js on that page (I can't use createTippy at the moment)? Just use a script tag leading to tippy.js (Where do I find that btw.?)?
Thank you very much for helping out!

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 18, 2024
This commit restores the state of the pull request from March 2023. The result looks different and still needs to be improved.
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 18, 2024
@github-actions github-actions bot added modifies/translation modifies/templates This PR modifies the template files labels May 18, 2024
@BlenderDefender
Copy link
Contributor Author

Alright, I've updated the files, but I know that it looks different compared to the original PR:
grafik

I'd greatly appreciate feedback to the changes made here and what I should improve.

@lunny lunny added this to the 1.23.0 milestone May 21, 2024
@BlenderDefender
Copy link
Contributor Author

Also, should I wait for #27931 to be merged in? It seems like it has a bigger impact on the code there and looks like it's probably going to be merged in sooner, since my PR still has some issues that I need to figure out and I have very limited time to work on it.

@lunny
Copy link
Member

lunny commented May 21, 2024

Also, should I wait for #27931 to be merged in? It seems like it has a bigger impact on the code there and looks like it's probably going to be merged in sooner, since my PR still has some issues that I need to figure out and I have very limited time to work on it.

I think so.

@BlenderDefender BlenderDefender deleted the branch go-gitea:main May 28, 2024 18:35
@BlenderDefender BlenderDefender deleted the main branch May 28, 2024 18:35
@GiteaBot GiteaBot removed this from the 1.23.0 milestone May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/templates This PR modifies the template files modifies/translation pr/wip This PR is not ready for review size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clone Button Rearrangement
9 participants