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

Remove conflicting CSS rules on notifications, improve notifications table #23565

Merged
merged 4 commits into from
Mar 21, 2023
Merged

Remove conflicting CSS rules on notifications, improve notifications table #23565

merged 4 commits into from
Mar 21, 2023

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 18, 2023 •
edited
Loading

Dropdowns on /notifications/subscriptions before and after:

Screenshot 2023-03-18 at 20 37 12

Screenshot 2023-03-18 at 20 41 29

These selectors are meant to target the notification list which I improved:

Screenshot 2023-03-19 at 01 52 11

Screenshot 2023-03-19 at 01 54 17

@silverwind silverwind added type/bug topic/ui Change the appearance of the Gitea UI labels Mar 18, 2023
@silverwind silverwind added this to the 1.20.0 milestone Mar 18, 2023
@silverwind silverwind added the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Mar 18, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 18, 2023
@silverwind silverwind changed the title Remove conflicting/unnecessary CSS rules on notifications Remove conflicting CSS rules on notifications, improve notifications table Mar 19, 2023
@silverwind silverwind requested a review from delvh March 19, 2023 00:54
@silverwind
Copy link
Member Author

silverwind commented Mar 19, 2023 •
edited
Loading

Did some more related changes to the notifications table:

  • Remove <span> wrapping on SVGs
  • Remove unnecessary padding on table
  • Fix active tab color
  • Fix blue icon color

Updated screenshots in OP.

@silverwind silverwind added the type/enhancement An improvement of existing functionality label Mar 19, 2023
@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 Mar 21, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 21, 2023
@wxiaoguang wxiaoguang mentioned this pull request Mar 21, 2023
Merged
@jolheiser jolheiser enabled auto-merge (squash) March 21, 2023 18:35
@jolheiser
Copy link
Member

🎺 🤖

@jolheiser jolheiser merged commit 253a00a into go-gitea:main Mar 21, 2023
@GiteaBot GiteaBot mentioned this pull request Mar 21, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 21, 2023
@silverwind @GiteaBot
Remove conflicting CSS rules on notifications, improve notifications …
c21e929 
…table (go-gitea#23565)

Dropdowns on `/notifications/subscriptions` before and after:

<img width="157" alt="Screenshot 2023-03-18 at 20 37 12"
src="https://user-images.githubusercontent.com/115237/226133906-e4ad6a0a-de24-4324-8e1d-94081d23fe85.png">
<img width="152" alt="Screenshot 2023-03-18 at 20 41 29"
src="https://user-images.githubusercontent.com/115237/226134038-c3946c32-a424-4b92-ad15-890e1036cafe.png">

These selectors are meant to target the notification list which I
improved:

<img width="1145" alt="Screenshot 2023-03-19 at 01 52 11"
src="https://user-images.githubusercontent.com/115237/226147907-1c35736a-4bc9-4698-9813-21a20a1d2106.png">
<img width="1148" alt="Screenshot 2023-03-19 at 01 54 17"
src="https://user-images.githubusercontent.com/115237/226147920-626dbd84-11d3-48db-a177-6d808e3212c0.png">
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Mar 21, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 21, 2023
@silverwind silverwind deleted the notif-rule branch March 21, 2023 20:37
lafriks pushed a commit that referenced this pull request Mar 21, 2023
@GiteaBot
Remove conflicting CSS rules on notifications, improve notifications …
9dfdfe2 
…table (#23565) (#23621)

Backport #23565 by @silverwind

Dropdowns on `/notifications/subscriptions` before and after:

<img width="157" alt="Screenshot 2023-03-18 at 20 37 12"
src="https://user-images.githubusercontent.com/115237/226133906-e4ad6a0a-de24-4324-8e1d-94081d23fe85.png">
<img width="152" alt="Screenshot 2023-03-18 at 20 41 29"
src="https://user-images.githubusercontent.com/115237/226134038-c3946c32-a424-4b92-ad15-890e1036cafe.png">

These selectors are meant to target the notification list which I
improved:

<img width="1145" alt="Screenshot 2023-03-19 at 01 52 11"
src="https://user-images.githubusercontent.com/115237/226147907-1c35736a-4bc9-4698-9813-21a20a1d2106.png">
<img width="1148" alt="Screenshot 2023-03-19 at 01 54 17"
src="https://user-images.githubusercontent.com/115237/226147920-626dbd84-11d3-48db-a177-6d808e3212c0.png">
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 22, 2023
@zjjhot
Merge remote-tracking branch 'upstream/main'
7095008 
* upstream/main:
  Use a general approch to improve a11y for all checkboxes and dropdowns. (go-gitea#23542)
  [skip ci] Updated translations via Crowdin
  Update PR documentation (go-gitea#23620)
  Set opaque background on markup and images (go-gitea#23578)
  Decouple the issue-template code from comment_tab.tmpl  (go-gitea#23556)
  Remove `id="comment-form"` dead code, fix tag (go-gitea#23555)
  Introduce path Clean/Join helper functions (go-gitea#23495)
  Remove conflicting CSS rules on notifications, improve notifications table (go-gitea#23565)
  Remove @metalmatze as maintainer (go-gitea#23612)
  Keep (add if not existing) xmlns attribute for generated SVG images (go-gitea#23410)
@wxiaoguang wxiaoguang mentioned this pull request Mar 23, 2023
Closed
wxiaoguang
wxiaoguang reviewed Mar 23, 2023
View reviewed changes
web_src/css/user.css
@@ -175,4 +154,13 @@

#notification_div .tab.segment {
overflow-x: auto;
padding: 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Made a comment on the backport PR, forward it here.

#23621 (comment)

The padding: 0 breaks the UI when there is no notification.

Copy link
Member Author

@silverwind silverwind Mar 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I wouldn't call it "break" but yes the "No items" div needs a bit more styling now that padding is removed.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, sorry I only know some English words. What do you think it should be called? I will follow your wording.

wxiaoguang
wxiaoguang reviewed Mar 23, 2023
View reviewed changes
web_src/css/user.css
}

#notification_div .menu .active.item {
background: var(--color-box-body);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the background should be set separately?

I guess all Dropdown menu items should share the same styles across the whole Gitea?

Copy link
Member Author

@silverwind silverwind Mar 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Dropdown? This is a tab directly attached on top of the table, quite unique I'd say.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought I do guess we could go for pull-style tabs here as well, like on the Releases/Tags page.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you only want to modify the "tab menu" styles, I think it's better to do #notification_div .tabular.menu

Otherwise, if there is a dropdown inside, its style is affected by .menu .active.item

@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

@HesterG HesterG HesterG approved these changes

@delvh delvh delvh approved these changes

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. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 topic/ui Change the appearance of the Gitea UI type/bug type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.
7 participants
@silverwind @jolheiser @wxiaoguang @HesterG @delvh @lunny @GiteaBot