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

Change green buttons to primary color #27099

Merged
merged 8 commits into from
Sep 18, 2023
Merged

Change green buttons to primary color #27099

merged 8 commits into from
Sep 18, 2023

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Sep 16, 2023 •
edited
Loading

I think it's better if the primary actions have primary color instead of green which fits better into the overall single-color UI design. This PR currently replaces every green button with primary:

Screenshot 2023-09-16 at 14 07 59 Screenshot 2023-09-16 at 14 07 51

Modal actions now use uncolored/primary instead of previous green/red colors. I also removed the box-shadow on all basic buttons:

Screenshot 2023-09-16 at 14 16 39 Screenshot 2023-09-16 at 14 17 42

The change currently includes the "Merge PR" button, for which we might want to make an exception to match the icon color there:

Screenshot 2023-09-16 at 14 33 53

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 16, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 16, 2023
@github-actions github-actions bot added the topic/ui Change the appearance of the Gitea UI label Sep 16, 2023
@puni9869
Copy link
Member

Could you share the screenshots in the dark theme.

@puni9869
Copy link
Member

Not blocking by screen shot.

puni9869
puni9869 approved these changes Sep 16, 2023
View reviewed changes
Copy link
Member

@puni9869 puni9869 left a comment

Choose a reason for hiding this comment

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

🚀
Let's roll.

@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 Sep 16, 2023
@puni9869
Copy link
Member

puni9869 commented Sep 16, 2023 •
edited
Loading

Probably skip-changelog label.

Need backport?

delvh
delvh approved these changes Sep 16, 2023
View reviewed changes
templates/repo/migrate/migrating.tmpl Show resolved Hide resolved
templates/repo/editor/edit.tmpl Show resolved Hide resolved
templates/repo/editor/patch.tmpl Show resolved Hide resolved
@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 Sep 16, 2023
@silverwind
Copy link
Member Author

Dark theme is unchanged because primary color is green there.

@puni9869
Copy link
Member

Dark theme is unchanged because primary color is green there.

Can't wait to see the change.

@silverwind
Copy link
Member Author

Would be interested in opinions regarding the "Merge PR" button as I'm undecided whether it should remain green.

@silverwind silverwind added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Sep 16, 2023
@lunny
Copy link
Member

lunny commented Sep 16, 2023

Could you also give some screenshots of arc-green theme?

@silverwind
Copy link
Member Author

silverwind commented Sep 17, 2023 •
edited
Loading

Could you also give some screenshots of arc-green theme?

See #27099 (comment)

Still waiting on opinions regarding the merge button. I think I'm personally leaning towards restoring green for it, just to match the green PR state icon.

@puni9869
Copy link
Member

I am okay with this button change.
No blocker from my side

If it ready to merge let's 🚀

wxiaoguang
wxiaoguang approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Some thoughts:

  1. maybe some buttons could be kept as green
  2. does it need to use "negative" instead of "red" in the future?

Overall LGTM

@silverwind silverwind marked this pull request as draft September 18, 2023 10:18
@silverwind
Copy link
Member Author

  1. Yes, at least Merge PR button should remain green, I will change it back.
  2. I guess negative/positive would be preferred to give themes the freedom to change these colors.

@puni9869
Copy link
Member

  1. Yes, at least Merge PR button should remain green, I will change it back.
  2. I guess negative/positive would be preferred to give themes the freedom to change these colors.

Point 1 is kind of trade off. Agreed.

@silverwind
Merge remote-tracking branch 'origin/main' into primary
9a23d22 
* origin/main:
  Fix the incorrect route path in the user edit page. (go-gitea#27007)
  Refactor lfs requests (go-gitea#26783)
  Display archived labels specially when listing labels (go-gitea#26820)
  Remove a `gt-float-right` and some unnecessary helpers (go-gitea#27110)
  [skip ci] Updated licenses and gitignores
  Fix token endpoints ignore specified account (go-gitea#27080)
  Make SSPI auth mockable (go-gitea#27036)
@silverwind
primary merge button
c714025
@silverwind
fix remaining cases of green buttons
83c335f
@silverwind silverwind marked this pull request as ready for review September 18, 2023 17:13
@silverwind
Copy link
Member Author

silverwind commented Sep 18, 2023 •
edited
Loading

Let's try with blue merge button then. Consistency is king.

I found around 20 more cases, now it should be definitely complete.

@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 18, 2023
@silverwind silverwind enabled auto-merge (squash) September 18, 2023 21:34
@silverwind silverwind merged commit 8099238 into go-gitea:main Sep 18, 2023
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Sep 18, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 18, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Sep 20, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
add8afe 
* giteaofficial/main:
  Improve actions docs related to `pull_request` event (go-gitea#27126)
  Remove outdated paragraphs when comparing Gitea Actions to GitHub Actions (go-gitea#27119)
  Fix: treat tab "overview" as "repositories" in user profiles without readme (go-gitea#27124)
  Fix incorrect test code for error handling (go-gitea#27139)
  Increase auth provider icon size on login page (go-gitea#27122)
  fix pagination for followers and following (go-gitea#27127)
  services/wiki: Close() after error handling (go-gitea#27129)
  Use fetch helpers instead of fetch (go-gitea#27026)
  Change green buttons to primary color (go-gitea#27099)
  Fix wrong xorm get usage on migration (go-gitea#27111)
  Fix the incorrect route path in the user edit page. (go-gitea#27007)
  Refactor lfs requests (go-gitea#26783)
  Display archived labels specially when listing labels (go-gitea#26820)
  Remove a `gt-float-right` and some unnecessary helpers (go-gitea#27110)
  [skip ci] Updated licenses and gitignores
  Fix token endpoints ignore specified account (go-gitea#27080)
  Make SSPI auth mockable (go-gitea#27036)
@delvh delvh modified the milestones: 1.22.0, 1.21.0 Sep 20, 2023
silverwind added a commit to silverwind/gitea that referenced this pull request Sep 21, 2023
@silverwind
Merge remote-tracking branch 'origin/main' into vitest-global
d8ee8ac 
* origin/main:
  Fix dropdown icon position (go-gitea#27175)
  Fix repo sub menu (go-gitea#27169)
  Fix review request number and add more tests (go-gitea#27104)
  Fix the variable regexp pattern on web page (go-gitea#27161)
  Fix organization field being null in POST /orgs/{orgid}/teams (go-gitea#27150)
  Add index to `issue_user.issue_id` (go-gitea#27154)
  [skip ci] Updated translations via Crowdin
  Start development on Gitea 1.22 (go-gitea#27155)
  Fix successful return value for `SyncAndGetUserSpecificDiff` (go-gitea#27152)
  Improve actions docs related to `pull_request` event (go-gitea#27126)
  Remove outdated paragraphs when comparing Gitea Actions to GitHub Actions (go-gitea#27119)
  Fix: treat tab "overview" as "repositories" in user profiles without readme (go-gitea#27124)
  Fix incorrect test code for error handling (go-gitea#27139)
  Increase auth provider icon size on login page (go-gitea#27122)
  fix pagination for followers and following (go-gitea#27127)
  services/wiki: Close() after error handling (go-gitea#27129)
  Use fetch helpers instead of fetch (go-gitea#27026)
  Change green buttons to primary color (go-gitea#27099)
  Fix wrong xorm get usage on migration (go-gitea#27111)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Dec 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@puni9869 puni9869 puni9869 approved these changes

@delvh delvh delvh approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. 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
Milestone
1.21.0
Development

Successfully merging this pull request may close these issues.
6 participants
@silverwind @puni9869 @lunny @wxiaoguang @delvh @GiteaBot