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

Number card spacing inconsistent in carousel #21

Closed
Nancy-Salpepi opened this issue Mar 15, 2023 · 3 comments
Closed

Number card spacing inconsistent in carousel #21

Nancy-Salpepi opened this issue Mar 15, 2023 · 3 comments
Assignees
Labels
type:bug Something isn't working

Comments

@Nancy-Salpepi
Copy link

Nancy-Salpepi commented Mar 15, 2023

Test device
MacBook Air M1 chip

Operating System
13.2.1

Browser
Safari

Problem description
For phetsims/qa#917, on the Lab screen:
Looking at the number carousel, the 10 is closer to the edge than the 1. The spacing between the 9 and 10 cards is also smaller than the space between all other cards. Not sure if this was intentional or not, to make up for the 10 card being wider than the others.

Visuals

Screenshot 2023-03-15 at 8 42 28 AM

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Number Compare‬ URL: https://phet-dev.colorado.edu/html/number-compare/1.0.0-dev.6/phet/number-compare_all_phet.html Version: 1.0.0-dev.6 2023-03-15 01:30:22 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15 Language: en-US Window: 1203x654 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
@Nancy-Salpepi Nancy-Salpepi added the type:bug Something isn't working label Mar 15, 2023
@chrisklus
Copy link
Contributor

Not sure if this was intentional or not, to make up for the 10 card being wider than the others.

Yes that's right, the way this carousel is organized is to have 10 items per page, so this much spacing is needed to fit 11-20 on the next page. I think we could have consistent spacing, but then there would be more than 10 items on the first page, and fewer than ten on the second. Or, 10 on the first page, and a total of 3 pages instead of 2. The difference in spacing feels like a good trade off to me so we have 2 pages with 10 options, but it would nice to confirm with @amanda-phet.

Thanks @Nancy-Salpepi!

@chrisklus chrisklus assigned amanda-phet and unassigned chrisklus Mar 16, 2023
@amanda-phet
Copy link
Contributor

@chrisklus is right. Other sims with differently sized objects in a carousel will also have varying spacing, since it's just a matter of fitting things in. I suppose we could evenly space 1-10 so there isn't less between 9-10 but I think it would actually look strangers, because while the space between would measure the same, the "centers" would start to look off since one object is wider. I am inclined to keep this as-is.

chrisklus added a commit to phetsims/number-suite-common that referenced this issue Mar 30, 2023
@chrisklus
Copy link
Contributor

@zepumph and I while working on #24 discovered that there was an easy AlignBox option to shift items in the carousel to the left of their containers. So, the single-digit cards with bigger xMargins have space to move, and the result is there is a consistent margin between cards on the first page of the carousel instead of the gap between the 9 and 10 being different. Also, the margin on the outside of the 1 card and 10 card is the same. And the the second page is unchanged with the smaller margin.

Here is how it looks:

image

@zepumph and I think this is a desirable change from this original issue that @Nancy-Salpepi opened and are good to close, anyone please comment if you disagree.

@chrisklus chrisklus reopened this Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants