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

Refix notification bell placement #20251

Merged
merged 2 commits into from
Jul 6, 2022
Merged

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Jul 5, 2022

The use of m-4 text black for the notification bell results in this
icon being shifted upwards. Instead we should use the item class but
adjust not-mobile and mobile-only to make their display: none
settings !important.

(As an aside: This is probably one of the only times we should use
!important in our less files and the rest should be avoided or
removed.)

Ref #20069
Revert #20236
Fix #20245

Signed-off-by: Andrew Thornton art27@cantab.net

The use of `m-4 text black` for the notification bell results in this
icon being shifted upwards. Instead we should use the `item` class but
adjust `not-mobile` and `mobile-only` to make their `display: none`
settings `!important`.

(As an aside: This is probably one of the only times we should use
`!important` in our less files and the rest should be avoided or
removed.)

Ref go-gitea#20069
Revert go-gitea#20236

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added topic/ui Change the appearance of the Gitea UI skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Jul 5, 2022
@zeripath zeripath added this to the 1.18.0 milestone Jul 5, 2022
@zeripath
Copy link
Contributor Author

zeripath commented Jul 5, 2022

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf

@tyroneyeh

Does this work for you?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 5, 2022
@tyroneyeh
Copy link
Contributor

Looks like it should be fine, thanks

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf

@tyroneyeh

Does this work for you?

let me do a clean rebuild after applying the patch.

this should then also fix #20245.

EDIT: works fine on my instance! 🚀

@zeripath
Copy link
Contributor Author

zeripath commented Jul 5, 2022

Yeah sorry I forgot you'd opened a bug from the comment!

@zeripath
Copy link
Contributor Author

zeripath commented Jul 5, 2022

I suspect that the !important should help other things too.

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

Yeah sorry I forgot you'd opened a bug from the comment!

np, thanks for the fix.

I suspect that the !important should help other things too.

it always does :D

Copy link
Contributor

Choose a reason for hiding this comment

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

works well, I'd officially approve if I could (apparently only maintainers can do that now)...

@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 Jul 5, 2022
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

works well, I'd officially approve if I could (apparently only maintainers can do that now)...

I'll do it for you :)

@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 Jul 6, 2022
@zeripath
Copy link
Contributor Author

zeripath commented Jul 6, 2022

make lgtm work

@zeripath zeripath merged commit 1e43a88 into go-gitea:main Jul 6, 2022
@zeripath zeripath deleted the re-fix-20069 branch July 6, 2022 17:33
zeripath added a commit to zeripath/gitea that referenced this pull request Jul 6, 2022
…itea#20236, go-gitea#20251)

Backport go-gitea#20108
Backport go-gitea#20236
Backport go-gitea#20251

Make notification bell more prominent on mobile

Co-authored-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Tyrone Yeh <siryeh@gmail.com>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@lunny
Copy link
Member

lunny commented Jul 6, 2022

LGTM

zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 7, 2022
* upstream/main:
  Modify milestone search keywords to be case insensitive (go-gitea#20266)
  Fix toolip on mobile notification bell (go-gitea#20270)
  Allow RSA 2047 bit keys (go-gitea#20272)
  Refix notification bell placement (go-gitea#20251)
  Bump mermaid from 9.1.1 to 9.1.2 (go-gitea#20256)
  EscapeFilter the group dn membership (go-gitea#20200)
  Only show Followers that current user can access (go-gitea#20220)
  Init popup for new code comment (go-gitea#20234)
  Bypass Firefox (iOS) bug (go-gitea#20244)
  Adjust max-widths for the repository file table (go-gitea#20243)
  Display full name (go-gitea#20171)
  Adjust class for mobile has the problem of double small bells (go-gitea#20236)
  Adjust template for go-gitea#20069 smallbell (go-gitea#20108)
  Add integration tests for the Gitea migration form (go-gitea#20121)
  Allow dev i18n to be more concurrent (go-gitea#20159)
  Allow enable LDAP source and disable user sync via CLI (go-gitea#20206)
6543 pushed a commit that referenced this pull request Jul 7, 2022
…) (#20269)

Backport #20108
Backport #20236
Backport #20251

Make notification bell more prominent on mobile

Co-authored-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Tyrone Yeh <siryeh@gmail.com>
Signed-off-by: Andrew Thornton <art27@cantab.net>
dineshsalunke pushed a commit to dineshsalunke/gitea that referenced this pull request Jul 9, 2022
The use of `m-4 text black` for the notification bell results in this
icon being shifted upwards. Instead we should use the `item` class but
adjust `not-mobile` and `mobile-only` to make their `display: none`
settings `!important`.

(As an aside: This is probably one of the only times we should use
`!important` in our less files and the rest should be avoided or
removed.)

Ref go-gitea#20069
Revert go-gitea#20236

Signed-off-by: Andrew Thornton <art27@cantab.net>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
The use of `m-4 text black` for the notification bell results in this
icon being shifted upwards. Instead we should use the `item` class but
adjust `not-mobile` and `mobile-only` to make their `display: none`
settings `!important`.

(As an aside: This is probably one of the only times we should use
`!important` in our less files and the rest should be avoided or
removed.)

Ref go-gitea#20069
Revert go-gitea#20236

Signed-off-by: Andrew Thornton <art27@cantab.net>
@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.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix for mobile view messed up non-mobile view
6 participants