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

Apply #12576 to top right reaction picker #12623

Merged
merged 2 commits into from
Aug 28, 2020

Conversation

CL-Jeremy
Copy link
Contributor

@CL-Jeremy CL-Jeremy commented Aug 27, 2020

I'll just open this since I cannot find a reason for the bug below.

After this PR, the reaction emoji is slightly shifted toward right in the top right drop-down. Seeing this in both Chromium (81) and Safari, but not in Firefox. Bottom left picker is unaffected.

Bildschirmfoto 2020-08-27 um 21 30 23

Bildschirmfoto 2020-08-27 um 21 32 52

Edit: confirmed to be Apple-specific. Should have something to do with the rendering engine.

@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2020

Codecov Report

Merging #12623 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #12623   +/-   ##
=======================================
  Coverage   43.44%   43.44%           
=======================================
  Files         645      645           
  Lines       71328    71328           
=======================================
+ Hits        30986    30989    +3     
+ Misses      35331    35328    -3     
  Partials     5011     5011           
Impacted Files Coverage Δ
models/unit.go 46.57% <0.00%> (-2.74%) ⬇️
modules/process/manager.go 72.50% <0.00%> (-2.50%) ⬇️
services/pull/pull.go 41.57% <0.00%> (-0.47%) ⬇️
modules/queue/workerpool.go 60.00% <0.00%> (+1.22%) ⬆️
modules/queue/unique_queue_disk_channel.go 55.38% <0.00%> (+1.53%) ⬆️
modules/log/event.go 59.43% <0.00%> (+1.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3bf1c4f...51a3a53. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 27, 2020
@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Aug 28, 2020
@lafriks lafriks added this to the 1.13.0 milestone Aug 28, 2020
@lafriks
Copy link
Member

lafriks commented Aug 28, 2020

how does it look in dark theme?

@CL-Jeremy
Copy link
Contributor Author

CL-Jeremy commented Aug 28, 2020

In Arc Green theme the only change is the rule for overriding highlight color on hover. The two popups would match exactly except for the bug I mentioned earlier.

Edit: more screenshots under macOS Catalina. Microsoft Edge shifts the emoji ever so slightly to the right, Safari to the left.

Bildschirmfoto 2020-08-28 um 13 33 28

Bildschirmfoto 2020-08-28 um 13 33 05

Bildschirmfoto 2020-08-28 um 13 37 12

@silverwind
Copy link
Member

I'm not sure what this change exactly fixes from the screenshots. Can you showcase a before/after comparison?

Last time I worked at that picker, I had the idea of rewriting it to flexbox because it's quite a hack currently, layout-wise.

@CL-Jeremy
Copy link
Contributor Author

As the title suggests, this PR adds hover effect to the picker in the top right corner.

For comparison, goto try.gitea.io and click on any of the top right picker icons.

The previous :hover styling targeted .select-reaction .item in .segment.reactions only, leaving out the top ones.

@silverwind
Copy link
Member

Ah I see. Totally missed the fact that there's two ways to trigger that menu.

@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 Aug 28, 2020
@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 Aug 28, 2020
@techknowlogick techknowlogick added skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug labels Aug 28, 2020
@techknowlogick techknowlogick merged commit 1fe9097 into go-gitea:master Aug 28, 2020
@CL-Jeremy CL-Jeremy deleted the reaction-picker-top branch August 29, 2020 12:16
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
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 type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants