Skip to content

Fix styling of label on GitHub Quickstart Page #5069

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.

Sign up for GitHub

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

Jump to bottom
Merged
matt-boris merged 1 commit into from
Jan 24, 2022
Merged

Fix styling of label on GitHub Quickstart Page #5069

matt-boris merged 1 commit into from
Jan 24, 2022

Conversation

@matt-boris
Copy link
Contributor

@matt-boris matt-boris commented Jan 21, 2022

Before

Screen Shot 2022-01-21 at 11 00 44 AM

  • Select/De-select any of the checkbox options show
  • Observe label becomes nearly unreadable due to styling change

Screen Shot 2022-01-21 at 11 03 37 AM

After

  • Label does not change style anymore when checkboxes are clicked

@matt-boris matt-boris added bug Something isn't working ui tc-web front end labels Jan 21, 2022
@matt-boris matt-boris requested a review from a team as a code owner January 21, 2022 16:15
@matt-boris matt-boris requested review from jwhitlock and petemoore and removed request for a team January 21, 2022 16:15
@petemoore
Copy link
Member

petemoore commented Jan 21, 2022

Thanks Boris!

This looks very reasonable to me, but I might not be the right person to review it, as I don't know much about front end stuff. Looking at https://mui.com/api/form-label/ I see that false is the default here, so I'm curious why it gets a different value when not specified, but mostly out of curiosity, because I too experience that when the window goes in and out of focus, the rendering changes (e.g. alt-tabbing between firefox and a different app) after a checkbox has been selected. Can you confirm that after selecting a checkbox, the rendering no longer changes when moving the focus to a different application? On mine, when I have firefox window in focus, the text is faint and unreadable, but if I move focus to a different app (e.g. Zoom) the text becomes readable again, and then moving focus back to firefox, is fades again (so is only readable when the window is not in focus).

@petemoore
Copy link
Member

petemoore commented Jan 21, 2022

The above CI failure is completely unrelated to this change, but is rather concerning:

[task 2022-01-21T16:17:06.186Z] Installing collected packages: taskgraph
[task 2022-01-21T16:17:06.187Z]   Running setup.py install for taskgraph: started
[task 2022-01-21T16:17:06.467Z]     Running setup.py install for taskgraph: finished with status 'done'
[task 2022-01-21T16:17:06.485Z] Successfully installed taskgraph-0.0.1
[task 2022-01-21T16:17:06.515Z] + PIP_IGNORE_INSTALLED=0
[task 2022-01-21T16:17:06.515Z] + pip install --user arrow taskcluster pyyaml
[task 2022-01-21T16:17:07.093Z] Collecting arrow
[task 2022-01-21T16:17:07.316Z]   Downloading https://files.pythonhosted.org/packages/ca/bc/ebc1afb3c54377e128a01024c006f983d03ee124bc52392b78ba98c421b8/arrow-0.17.0-py2.py3-none-any.whl (50kB)
[task 2022-01-21T16:17:07.352Z] Collecting taskcluster
[task 2022-01-21T16:17:07.526Z]   Downloading https://files.pythonhosted.org/packages/c5/86/b971767fae850e3d6daa894e3f429e31f459a2159bb07e97537de28fa8e1/taskcluster-44.4.0.tar.gz (106kB)
[task 2022-01-21T16:17:07.757Z]     Complete output from command python setup.py egg_info:
[task 2022-01-21T16:17:07.757Z]     Traceback (most recent call last):
[task 2022-01-21T16:17:07.757Z]       File "<string>", line 1, in <module>
[task 2022-01-21T16:17:07.757Z]       File "/tmp/pip-build-S832Dc/taskcluster/setup.py", line 64, in <module>
[task 2022-01-21T16:17:07.757Z]         with open('README.md', encoding='utf8') as f:
[task 2022-01-21T16:17:07.757Z]     TypeError: 'encoding' is an invalid keyword argument for this function
[task 2022-01-21T16:17:07.757Z]     
[task 2022-01-21T16:17:07.757Z]     ----------------------------------------

@petemoore
Copy link
Member

petemoore commented Jan 21, 2022

We removed support for python 2 in release 44.4.0, and it looks like something depends on it that we didn't catch. :-(

@jwhitlock jwhitlock mentioned this pull request Jan 21, 2022
Closed
@matt-boris
Copy link
Contributor Author

matt-boris commented Jan 21, 2022

@petemoore

Looking at https://mui.com/api/form-label/ I see that false is the default here, so I'm curious why it gets a different value when not specified...

It seems like the FormGroup component sets this focused prop for some reason. You can see their example behaves identically to ours, except for the fact that our label becomes unreadable. I'm not too sure why this is, but I decided to just try hardcoding it to false, and that's what worked.

Can you confirm that after selecting a checkbox, the rendering no longer changes when moving the focus to a different application?

Yep! Label does not change at all with this fix implemented. You can also get the style back to "readable" if you click somewhere else on the page, like the label text, for example.

petemoore
petemoore reviewed Jan 21, 2022
View reviewed changes
Copy link
Member

@petemoore petemoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Boris!

@jwhitlock Do you want to have a look too?

@petemoore
Copy link
Member

petemoore commented Jan 21, 2022

The above CI failure is completely unrelated to this change, but is rather concerning:

[task 2022-01-21T16:17:06.186Z] Installing collected packages: taskgraph
[task 2022-01-21T16:17:06.187Z]   Running setup.py install for taskgraph: started
[task 2022-01-21T16:17:06.467Z]     Running setup.py install for taskgraph: finished with status 'done'
[task 2022-01-21T16:17:06.485Z] Successfully installed taskgraph-0.0.1
[task 2022-01-21T16:17:06.515Z] + PIP_IGNORE_INSTALLED=0
[task 2022-01-21T16:17:06.515Z] + pip install --user arrow taskcluster pyyaml
[task 2022-01-21T16:17:07.093Z] Collecting arrow
[task 2022-01-21T16:17:07.316Z]   Downloading https://files.pythonhosted.org/packages/ca/bc/ebc1afb3c54377e128a01024c006f983d03ee124bc52392b78ba98c421b8/arrow-0.17.0-py2.py3-none-any.whl (50kB)
[task 2022-01-21T16:17:07.352Z] Collecting taskcluster
[task 2022-01-21T16:17:07.526Z]   Downloading https://files.pythonhosted.org/packages/c5/86/b971767fae850e3d6daa894e3f429e31f459a2159bb07e97537de28fa8e1/taskcluster-44.4.0.tar.gz (106kB)
[task 2022-01-21T16:17:07.757Z]     Complete output from command python setup.py egg_info:
[task 2022-01-21T16:17:07.757Z]     Traceback (most recent call last):
[task 2022-01-21T16:17:07.757Z]       File "<string>", line 1, in <module>
[task 2022-01-21T16:17:07.757Z]       File "/tmp/pip-build-S832Dc/taskcluster/setup.py", line 64, in <module>
[task 2022-01-21T16:17:07.757Z]         with open('README.md', encoding='utf8') as f:
[task 2022-01-21T16:17:07.757Z]     TypeError: 'encoding' is an invalid keyword argument for this function
[task 2022-01-21T16:17:07.757Z]     
[task 2022-01-21T16:17:07.757Z]     ----------------------------------------

John is working on a fix for this in #5071.

@matt-boris matt-boris force-pushed the matt-boris/githubQuickstartStylingBug branch 2 times, most recently from 48fc53d to 9d81c41 Compare January 21, 2022 21:21
@matt-boris matt-boris force-pushed the matt-boris/githubQuickstartStylingBug branch from 9d81c41 to a19d6c1 Compare January 24, 2022 14:38
jwhitlock
jwhitlock approved these changes Jan 24, 2022
View reviewed changes
Copy link
Contributor

@jwhitlock jwhitlock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @matt-boris, looks good to me!

@matt-boris matt-boris merged commit 894aed2 into main Jan 24, 2022
@matt-boris matt-boris deleted the matt-boris/githubQuickstartStylingBug branch January 24, 2022 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petemoore petemoore petemoore left review comments

+1 more reviewer

@jwhitlock jwhitlock jwhitlock approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working ui tc-web front end

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@matt-boris @petemoore @jwhitlock