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

Tweak repo sidebar #32847

Merged
merged 21 commits into from
Dec 15, 2024
Merged

Tweak repo sidebar #32847

merged 21 commits into from
Dec 15, 2024

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Dec 15, 2024

Before and after:

Screenshot 2024-12-15 at 04 53 53 Screenshot 2024-12-15 at 04 53 41

Diff without whitespace: https://github.com/go-gitea/gitea/pull/32847/files?diff=unified&w=1

The tw-mt-2 is fine even if the element renders empty:

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 15, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 15, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Dec 15, 2024
@silverwind silverwind marked this pull request as draft December 15, 2024 04:06
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 15, 2024

By reading the tmpl code again, I can see the "flex-list" was already abused.

Ideally it should be changed to a layout like this:

<div class="flex-list">
    <flex-item>
         <flex-item-main> (this "flex-item-main" might be omitted since there is no "leading icon", not quite sure)
              <flex-item-title> Description (the section title)</>
              <flex-item-body with other styles> 
                    description
                    topic
                    any other elements etc ....
              </>
          </>
     </>
</>

@silverwind
Copy link
Member Author

silverwind commented Dec 15, 2024

Did some tweaks:

image
  • Link wrapper div is now flex-item-body
  • Individual links are flex-text-block
  • Tweaked homepage link so it's colored like GitHub's
  • Font size override to 16px is removed, description is now 14px

Feel free to do more tweaks.

@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 15, 2024
@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 15, 2024
@wxiaoguang
Copy link
Contributor

Thank you, will correct the "flex-list" based on your work

@silverwind
Copy link
Member Author

One tiny tweak added, gap between topic labels is reduced. I'll let you finish now.

image

@silverwind silverwind changed the title Tweak sidebar link margin Tweak repo sidebar Dec 15, 2024
@silverwind silverwind marked this pull request as ready for review December 15, 2024 04:29
@wxiaoguang
Copy link
Contributor

Force pushed (because too many changes) and applied your "gap-1" for the labels.

@silverwind
Copy link
Member Author

silverwind commented Dec 15, 2024

Bug: label text is too dull because of color: inherit:

image

Otherwise LGTM.

@silverwind
Copy link
Member Author

Fix pushed:

image

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 15, 2024
@silverwind
Copy link
Member Author

silverwind commented Dec 15, 2024

Did more tweaks:

image
  • Extracted release label into its own subtemplate. This was previously always showing untranslated "latest" (i18n key was missing) while the release itself would show "Stable" on the release page. Now it shows "Latest" on both pages and the translation for that is added.
  • Restored 16px font size for description block, it fits better with latest release which also has 16px from .flex-item-title.
  • Moved the repo-home-sidebar-bottom and repo-home-sidebar-top divs into their respective templates.
  • Reduce overly specific css selectors.

@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 15, 2024
@wxiaoguang
Copy link
Contributor

  • Extracted release label into its own subtemplate. This was previously always showing untranslated "latest" (i18n key was missing) while the release itself would show "Stable" on the release page. Now it shows "Latest" on both pages and the translation for that is added.

That's not right. There could be many "stable" but only one "latest"

image

@silverwind
Copy link
Member Author

silverwind commented Dec 15, 2024

Hmm so we need to somehow adjust the label template to detect latest vs. stable. FWIW, GitHub has no notion of stable. They just show no label on non-latest non-prerelease/draft.

@wxiaoguang
Copy link
Contributor

Hmm so we need to somehow adjust the label template to detect latest vs. stable.

I would suggest do not do that trick in this PR.

Leave the "latest" on repo home, and leave the "stable" for the list.

@silverwind
Copy link
Member Author

OK:

Repo:
image

Release:
image

@silverwind
Copy link
Member Author

silverwind commented Dec 15, 2024

Ideally this IsLatest attribute would be set as .Release.IsLatest in backend, but I have no idea where to change that.

@github-actions github-actions bot removed the modifies/go Pull requests that update Go code label Dec 15, 2024
@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 Dec 15, 2024
@wxiaoguang
Copy link
Contributor

Ideally this IsLatest attribute would be set as .Release.IsLatest in backend, but I have no idea where to change that.

Since the time for releasing 1.23 is limited, I would suggest we only do necessary changes at the moment, keep 1.23 release stable. Leave trivial (or hard) problem to the future.

@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 Dec 15, 2024
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 15, 2024
@silverwind silverwind enabled auto-merge (squash) December 15, 2024 09:40
@silverwind silverwind merged commit df9a78c into go-gitea:main Dec 15, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 15, 2024
@silverwind silverwind deleted the sidebarmargin branch January 15, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/frontend modifies/templates This PR modifies the template files modifies/translation size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants