Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

De-emphasize signed commits#31160

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
wxiaoguang merged 40 commits intogo-gitea:mainfromBlenderDefender:signed-commits-ui
Dec 27, 2024

Conversation

@BlenderDefender
Copy link
Contributor

@BlenderDefenderBlenderDefender commentedMay 29, 2024•
edited by wxiaoguang
Loading

Update: the latest change still uses the old layout, the signed commits are still made to be de-emphasized a little (it won't affect its parents, for example: the commit details page), I didn't change their colors.

And the new code structure is easier to make more improvements or refactor, for example: change the colors to de-emphasize more, or design some new layouts.

screenshot

image

Old description:

This pull request changes how signed commits are displayed, as signed commits were emphasized too much previously. This includes a rewrite of the commit list UI, which now uses an unordered list instead of a table. If a commit is signed, a clickable badge is shown, indicating that the commit is signed with either a verified or an unverified signature.

fixes#29641

Old Comparison

Latest commit

Before

grafik

After

grafik

Commit list

Before

grafik

After

grafik

Commit details

Before

grafik

After

grafik
grafik

Commit Graph

Before

grafik

After

grafik

Clicking on the badge opens the following modal:
grafik
grafik
grafik

lunny, Frankkkkk, silverwind, yp05327, ExplodingDragon, Sirherobrine23, splitt3r, Jokiller230, stuzer05, and lutinglt reacted with thumbs up emoji
Rework the signed commits UI to show a clickable badge for signed commits. Clicking this badge reveals further information about the trust status of the signature.
@GiteaBotGiteaBot added the lgtm/need 2This PR needs two approvals by maintainers to be considered for merging. labelMay 29, 2024
@github-actionsgithub-actionsbot added the modifies/templatesThis PR modifies the template files labelMay 29, 2024
@techknowlogick
Copy link
Member

Thanks!! 🙏

This is so helpful. The visual changes look great!

I think the CI changes may be related, perhaps due to an undefined variable previously wrapped in a guard conditional.

My main development machine is undergoing some repairs, and so I can't properly go through this changeset for review right now, but if you'd like for me to determine the cause of the CI fail I can do that for you once I'm back on my main device.

Frankkkkk reacted with thumbs up emojiBlenderDefender reacted with heart emoji

@BlenderDefender
Copy link
ContributorAuthor

@techknowlogick That would be really helpful. Do you know who I can ask to review this PR?

@silverwind
Copy link
Member

silverwind commentedMay 30, 2024•
edited
Loading

Looks nice based on the screenshots. I had started only on the commit detail page in#30221, but this looks more complete. Maybe you can take some inspiration from my PR.

You will definitely want to include the removal of unused CSS here.

@BlenderDefender
Copy link
ContributorAuthor

@silverwind Looks like there are quite a few styles that will be removed with this... TheisSigned class isn't used anymore, so all styles with that selector can be removed.
grafik

@silverwind
Copy link
Member

silverwind commentedMay 30, 2024•
edited
Loading

@silverwind Looks like there are quite a few styles that will be removed with this... TheisSigned class isn't used anymore, so all styles with that selector can be removed.

Indeed, please remove all CSS that has theisSigned class in it. I did similarhere but am not sure if my removal there is complete, so remove until a search forisSigned does not match anywhere in the CSS files anymore.

BlenderDefender reacted with thumbs up emoji

This commit removes all classnames (like for example `ui text`) that don't have any effect on the elements they're applied to.
@silverwind
Copy link
Member

silverwind commentedMay 30, 2024•
edited
Loading

One request: Instead of

image

could we use this SVG icon:

image

If you want to show some additional text, it can be done withdata-tooltip-content on the element. Again, you can take a hint from my PR, the icon logicis here and we should extract it into a subtemplate.

These styles aren't used anymore, so they can be safely deleted
@BlenderDefender
Copy link
ContributorAuthor

Would look like this, I'm not quite sure about the warning icon in light mode though:
grafik
grafik
grafik
grafik

@silverwind
Copy link
Member

Hmm, after seeing GitHub also has only text labels in these places, let's keep the text labels. One change I would like to see is normal border radius (tw-rounded), then I think it's fine. Should look similar to this:

image
BlenderDefender reacted with thumbs up emoji

@GiteaBotGiteaBot added lgtm/need 1This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2This PR needs two approvals by maintainers to be considered for merging. labelsJun 12, 2024
@delvh
Copy link
Member

@silverwind did you have time to fix these hacks?

@lunnylunny modified the milestones:1.23.0,1.24.0Sep 24, 2024
@wxiaoguang
Copy link
Contributor

I still think we can fix them without such hacks, I will check later but feel free to investigate yourself in the meanwhile.

@silverwind@BlenderDefender how to continue?

# Conflicts:#templates/repo/commit_page.tmpl#templates/repo/commits_list.tmpl#templates/repo/commits_list_small.tmpl#templates/repo/graph/commits.tmpl#templates/repo/latest_commit.tmpl#templates/repo/shabox_badge.tmpl#web_src/css/repo.css
@wxiaoguang
Copy link
Contributor

Manged to fix some problems. And FYI: the modal can't be used, the reason is complicated but since this PR is stale for long time, I won't spend time on explaining it unless there are interests.

@wxiaoguangwxiaoguang marked this pull request as draftDecember 27, 2024 18:12
@wxiaoguangwxiaoguang marked this pull request as ready for reviewDecember 27, 2024 19:08
@GiteaBotGiteaBot added lgtm/doneThis PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1This PR needs approval from one additional maintainer to be merged. labelsDec 27, 2024
@wxiaoguangwxiaoguang merged commit079a1ff intogo-gitea:mainDec 27, 2024
26 checks passed
@wxiaoguang
Copy link
Contributor

Since this PR now also blocks others, so I merge it after CI passes. If there is anything wrong, I will also fix in first time.

And there could be more fine-tunes to the colors to de-emphasize more

zjjhot added a commit to zjjhot/gitea that referenced this pull requestDec 30, 2024
* giteaofficial/main:  Refactor pagination (go-gitea#33037)  Test webhook email (go-gitea#33033)  Fix review code comment avatar alignment (go-gitea#33031)  Fix templating in pull request comparison (go-gitea#33025)  Refactor tests (go-gitea#33021)  [skip ci] Updated translations via Crowdin  always show assignees on right (go-gitea#33006)  fix toggle commit body button ui when latest commit message is long (go-gitea#32997)  Fix and/or comment some legacy CSS problems (go-gitea#33015)  Refactor comment history and fix content edit (go-gitea#33018)  Fix bug on activities (go-gitea#33008)  Refactor arch route handlers (go-gitea#32993)  fix scoped label ui when contains emoji (go-gitea#33007)  [skip ci] Updated translations via Crowdin  De-emphasize signed commits (go-gitea#31160)  Fix eslint (go-gitea#33002)  Fix Agit pull request permission check (go-gitea#32999)  Support for email addresses containing uppercase characters when activating user account (go-gitea#32998)
@go-giteago-gitea locked asresolvedand limited conversation to collaboratorsMar 27, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@silverwindsilverwindsilverwind left review comments

@wxiaoguangwxiaoguangwxiaoguang approved these changes

@delvhdelvhdelvh approved these changes

Assignees

No one assigned

Labels

lgtm/doneThis PR has enough approvals to get merged. There are no important open reservations anymore.modifies/goPull requests that update Go codemodifies/jsmodifies/templatesThis PR modifies the template files

Projects

None yet

Milestone

1.24.0

Development

Successfully merging this pull request may close these issues.

De-emphasize signed commits

7 participants

@BlenderDefender@techknowlogick@silverwind@delvh@wxiaoguang@lunny@GiteaBot
[8]ページ先頭
©2009-2025 Movatter.jp