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

Short Strings are centered #59

Closed
KatieWoe opened this issue Feb 12, 2019 · 12 comments
Closed

Short Strings are centered #59

KatieWoe opened this issue Feb 12, 2019 · 12 comments
Assignees

Comments

@KatieWoe
Copy link
Contributor

For phetsims/qa#281
From slack:

Me:
centered
Are they supposed to be centered like this?
I feel like this was an issue too
But I can't find it.

Amy Rouinfar [10:26 AM]
No, the padding to the left of the checkbox and to the right of the icons should never change
Essentially, there would be a big gap between the “x” and the icon.
Slack Denzell about this one. I don’t remember this issue, specifically.

Me:
I just checked, and this is happening in the published version of MAS.

Denzell Barnett [10:31 AM]
I don’t recall a specific issue for this checkbox. Checking…
Hmm not finding any specific issues. Could you create a new issue for this stringTest?

@arouinfar
Copy link
Contributor

It looks like the checkbox/icon group is centered in the panel. However, the checkboxes/strings should remain on the left side of the panel, and the icons should remain on the right side of the panel. Making the strings very short should result in a larger gap between the string and icon.

@arouinfar arouinfar removed their assignment Feb 12, 2019
@Denz1994
Copy link
Contributor

The fix for the checkbox described above should be applied to all checkbox items. This includes VectorVisibilityControlNode and IndicatorVisibilityControlNode for both basics and non-basics versions.

@KatieWoe
Copy link
Contributor Author

And the Gravity string seems a bit short @arouinfar
longer

@arouinfar
Copy link
Contributor

And the Gravity string seems a bit short @arouinfar

Thanks @KatieWoe, since this is a maxWidth issue, and not really related to ?stringTest=x, I've moved #59 (comment) to #63.

@Denz1994
Copy link
Contributor

Denz1994 commented Mar 8, 2019

Only vectors panel has been updated with some logic to assign a spacing dynamically, based on the content of the panel. Before I apply these changes elsewhere and continue with refactors, could you please review the behavior of this panel with the arrow vectors on master @arouinfar.

Here is what the vector panel looks like with stringTest=x:
image

@Denz1994
Copy link
Contributor

Denz1994 commented Mar 8, 2019

Note any fix for the original panel described in #59 (comment) should be applied to the vector panel as well, so it is safe to review both.

@arouinfar
Copy link
Contributor

@Denz1994 the vector panel has the desired alignment with ?stringTest=x. Ideally, the reference line checkboxes would be left aligned with the vector checkboxes, and the reference line icons would be right-aligned with the vector icons.

Currently, the reference line checkboxes are still centered in master.

@Denz1994
Copy link
Contributor

Denz1994 commented Mar 12, 2019

the vector panel has the desired alignment with ?stringTest=x

Nice. Similar behavior for a dynamic maxWidth was applied to all other checkboxes and radio buttons with a complimentary icon. Could you review master @arouinfar and assign back to me?

@Denz1994 Denz1994 assigned arouinfar and unassigned Denz1994 Mar 12, 2019
@arouinfar
Copy link
Contributor

Looks good in MAS and MASB! Thanks @Denz1994.

@Denz1994
Copy link
Contributor

The relevant commits need to be cherry-picked into the RC branch for MASB before the next RC test.

@Denz1994
Copy link
Contributor

Relevant commits have been cherry-picked into MAS masses-and-springs-basics-1.0 branch and MASB 1.0 branch. This is now ready for verification in the next RC test.

@KatieWoe
Copy link
Contributor Author

Looks good in both versions. MASB 1.0.0-rc.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants