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

Overlapping of icon-background on hover #974

Closed
rucha0702 opened this issue Jan 25, 2022 · 12 comments
Closed

Overlapping of icon-background on hover #974

rucha0702 opened this issue Jan 25, 2022 · 12 comments
Labels
enhancement to be release Fixed, to be release

Comments

@rucha0702
Copy link
Contributor

web-activity-icons overlap a little with the icons near them on hovering.

sug1 sug2

sug3

However, it looks fine if the border-radius is set to 50%

ans1

sug-ans_Trim.mp4

I would be happy to create a PR if the issue is approved.

@llaske
Copy link
Owner

llaske commented Jan 30, 2022

You're right there is an overlap on icons in "circle mode" (i.e. when there is no size to draw all icons in the same page as a spiral).
There is no overlap in spiral mode.

Peek 30-01-2022 10-39

I don't like the idea to change border radius, it's not consistent with the Sugar UI.
May be the idea could be to add spaces between icons in circle mode.

FYI all magic regarding spiral/circle mode happen here.

@rucha0702
Copy link
Contributor Author

Hey, so I went through the homeview.js file you mentioned above and found a constant by the name of iconSpacingFactor over
here that defines the spacing. After a small change in its value, it looked like this.

Before:
before

After:
between

Am I doing it right?

@llaske
Copy link
Owner

llaske commented Feb 1, 2022

Am I doing it right?

Just changing this value will probably affect spacing for both spiral and circle mode.
It will be better to find a solution to fix only the circle mode issue.

@rucha0702
Copy link
Contributor Author

Am I doing it right?

Just changing this value will probably affect spacing for both spiral and circle mode. It will be better to find a solution to fix only the circle mode issue.

Oh okay, I'll try finding a solution just for the circle mode.

@AnandChourasia007
Copy link
Contributor

AnandChourasia007 commented Feb 25, 2022

Hey @llaske , I think I fixed this issue without affecting the spiral design.
Before
Before fix
:
After
Hover overlap fixed without affecting the spiral design
:
Spiral mode view after changing circular mode view:
Spiral view after making changes to the circular view

Is this good?

@llaske
Copy link
Owner

llaske commented Feb 25, 2022

@AnandChourasia007 Sounds good.
Please send a PR so I will be able to review it.

@AnandChourasia007
Copy link
Contributor

@llaske I have submitted the PR to the repo from which I had forked which was sugarlabs/sugarizer. After that I saw that most PR have been made to your repo ilaske/sugarizer. Is this fine or should I fork from your repo and submit my PR ?

@llaske
Copy link
Owner

llaske commented Feb 25, 2022

@AnandChourasia007 the latest repo is https://github.com/llaske/sugarizer . Please send your PR on the dev branch.

@AnandChourasia007
Copy link
Contributor

Yeah sure! Sorry for the trouble.

@AnandChourasia007
Copy link
Contributor

Hey @llaske, after making the fix, to how much extent should I check if the hover effect has been fixed? I mean do I have to ensure that this fixes hover effect all the way down upto mobile phones screen dimensions? Taking mobile responsiveness into account I mean. Because that would require resizing appropriately the inner two icons as well.
This is the current extent to which there is no overlap after fixing:
Fix attempt 2
Is this good? Or should I fix for even more tighter screens.

@llaske
Copy link
Owner

llaske commented Feb 26, 2022

@AnandChourasia007 You could find a good description of responsive limit of Sugarizer here.

@llaske
Copy link
Owner

llaske commented Mar 3, 2022

Fixed in 30c4a37

@llaske llaske added the to be release Fixed, to be release label Mar 3, 2022
@llaske llaske closed this as completed in a63bb18 Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement to be release Fixed, to be release
Projects
None yet
Development

No branches or pull requests

3 participants