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

improve explore relevancy note #22421

Merged

Conversation

crystalcommunication
Copy link
Contributor

makes the explore relevancy note much prettier on most themes

@crystalcommunication
Copy link
Contributor Author

crystalcommunication commented Jan 13, 2023

The relevancy note box in the "Explore" section applies the "blue" class to the div element which seem to paint a blueish color to the background of the component, this may be undesirable depending on the user's theme.
The same box has inconsistency in the corners: rounded on top but sharp on bottom.
I'd also suggest detaching it from the search box, since the corners of the two boxes, being rounded, don't align correctly and don't look particularly good.

After and before in the screenshots, both with Forgejo dark theme.

Originally posted by @Gaarco in https://codeberg.org/Codeberg/Community/issues/885#issue-234675

before
image

after
image

requesting review from @Gusted

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 13, 2023
@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 Jan 13, 2023
@@ -30,7 +30,7 @@
</div>
</form>
{{if .OnlyShowRelevant}}
<div class="ui blue attached message explore-relevancy-note">
<div class="ui message explore-relevancy-note">
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 class is not removed since the ui.explore-relevancy-note was deleted in _explore.less ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess I thought it could be useful if a theme wanted to explicitly change the styling on it independently. I'm not particularly attached to it.

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.

As you showed in
, is it really a good idea to remove the max-width restriction?
I think that style looked better than what you are proposing.

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.

On the other hand, it is no longer attached so it does look fine.

@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 Jan 13, 2023
@crystalcommunication
Copy link
Contributor Author

I made the changes as proposed in the comment that I copied from Codeberg. I think it would look awkward being detached and also the same width as the search bar. When it was attached, @Gaarco made a great point about the borders being slightly misaligned and the rounded corners making it look weird if you pixel-peep. If you prefer it attached to the search box, perhaps we can adjust the styling to eliminate these issues instead.

@jolheiser jolheiser merged commit f1c13b4 into go-gitea:main Jan 13, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 13, 2023
* upstream/main:
  Continue GCing other repos on error in one repo (go-gitea#22422)
  improve explore relevancy note (go-gitea#22421)
  fix: don't replace err variable in nested check (go-gitea#22416)
  Add more packages to denylist (go-gitea#22412)
  fix wrong theme class when logged out if default theme is changed (go-gitea#22408)
  Update golang deps (go-gitea#22410)
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants