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

Remove Radium from JavalabSettings #47463

Merged
merged 3 commits into from
Aug 4, 2022

Conversation

bencodeorg
Copy link
Contributor

@bencodeorg bencodeorg commented Aug 2, 2022

JavalabSettings passes an object with a pseudoclass key (:hover) to JavalabButton, but does not require Radium itself, as I understand it? It'd probably be good to have someone double check my understanding though. I also remove the optional style prop, which isn't used (just to clean up/hopefully hedge against future style prop passing).

Testing story

Tested manually that JavalabSettings (the "Settings" button at the bottom right of the Javalab console) continued to function as expected.

@bencodeorg bencodeorg requested a review from a team August 2, 2022 23:31
Copy link
Contributor

@maddiedierker maddiedierker left a comment

Choose a reason for hiding this comment

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

JavalabSettings passes an object with a pseudoclass key (:hover) to JavalabButton, but does not require Radium itself, as I understand it?

looking at this component, that's my understanding as well. it looks like the only styles that require radium are being passed to JavalabButton, which loads radium itself.

also, great idea to remove the unused style prop! we'll need to slowly move away from that, so this is awesome to see.

Base automatically changed from ben/javalab-file-explorer-remove-radium to staging August 4, 2022 15:40
@bencodeorg bencodeorg merged commit 0ba3673 into staging Aug 4, 2022
@bencodeorg bencodeorg deleted the ben/javalab-settings-remove-radium branch August 4, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants