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

Keyboard dialog columns and content #136

Closed
terracoda opened this issue May 23, 2019 · 4 comments
Closed

Keyboard dialog columns and content #136

terracoda opened this issue May 23, 2019 · 4 comments

Comments

@terracoda
Copy link

@zepumph, at our last meeting I asked about rearranging the Keyboard Shortcuts dialog content. We decided not to make any changes.

However, I think I wasn't clear on what I actually wanted. I want the PDOM reading order to stay the same, but for the second column to be the long one.

The layout would be similar to the dialog in Molarity, having both Change Mass instructions and Basic Actions in the second column, like this:
kb-shortcuts-column2_web

If I am correct in how I think the columns are read out, I think this layout visually puts the important content up top and does not affect the PDOM order.

This is not a blocking issue. Do it if you have time and only if the layout does not change the PDOM order.

I am marking as low priority.

@zepumph
Copy link
Member

zepumph commented May 23, 2019

Ahh! That makes more sense. And it is very easy. Implemented above, please review.

@terracoda
Copy link
Author

Beautiful!
Could you verify that the first 2 headings, "Move Spheres" and "Change Mass" are starting at the same spot? "Move Spheres" looks a tiny bit higher to me, but maybe it is an optical illusion.

In addition, to reduce vertical height of the entire dialog, you could reduce some of the space above "Basic Actions" heading, but only if possible. I am assuming this chunk is from common code, so might not be possible to reduce.

@terracoda
Copy link
Author

@zepumph, if this spacing is too nit-picky, it is not a blocking issue. We can just as easily close this issue.

@terracoda terracoda removed their assignment May 24, 2019
@zepumph
Copy link
Member

zepumph commented May 25, 2019

Could you verify that the first 2 headings, "Move Spheres" and "Change Mass" are starting at the same spot? "Move Spheres" looks a tiny bit higher to me, but maybe it is an optical illusion.

In the code they are both "top aligned" and I actually thought that "Change Mass" was higher when looking at it with a ruler (lol). I think it is an optical illusion.

In addition, to reduce vertical height of the entire dialog, you could reduce some of the space above "Basic Actions" heading, but only if possible. I am assuming this chunk is from common code, so might not be possible to reduce.

I think that the spacing is based on the default of Dialog in general, and would probably be best to keep it.

I'm going to close. Let me know if you feel strongly about these two things.

@zepumph zepumph closed this as completed May 25, 2019
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

2 participants