Skip to content

/ ghostery-extension

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

Jump to bottom

GH-1967 Themes Panel Update #510

Merged
merged 7 commits into from Apr 14, 2020
+234 −46
Merged

GH-1967 Themes Panel Update #510

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
317ab64
Replace themes toggle with radio buttons
benstrumeyer Mar 11, 2020
f050b83
Center inner circle in radio buttons
benstrumeyer Mar 11, 2020
e8d7f92
Merge branch 'develop' into GH-1967/themes-panel-update
benstrumeyer Mar 12, 2020
f9b1546
Remove console error
benstrumeyer Mar 12, 2020
2a8bce2
Clean up radio button code
benstrumeyer Mar 31, 2020
d0ec9be
Merge branch 'develop' into GH-1967/themes-panel-update
benstrumeyer Mar 31, 2020
c90b51a
Merge branch 'develop' into GH-1967/themes-panel-update
christophertino Apr 14, 2020
File filter
Filter file types
Clear filters
Jump to
Jump to file
Failed to load files.

Always

Just for now

Prev Next
Loading status checks…

Remove console error

  • Loading branch information
@benstrumeyer
benstrumeyer committed Mar 12, 2020
commit f9b154628d18d30ee46099f397b0340b30a0cf74
4 app/panel/components/BuildingBlocks/RadioButtonGroup.jsx
View file
Open in desktop
@@ -50,12 +50,12 @@ class RadioButtonGroup extends React.Component {
const { buttons } = this.state;
return (
this.props.items.map((item, index) => (
<div className="flex-container align-justify RadioButtonGroup__container">
<div className="flex-container align-justify RadioButtonGroup__container" key={`${index * 2}`}>
<span className="RadioButtonGroup__label">
{t(item.text)}
</span>
<div>
<RadioButton key={buttons[index]} checked={buttons[index]} handleClick={() => this.handleClick(index)} />
<RadioButton checked={buttons[index]} handleClick={() => this.handleClick(index)} />
This conversation was marked as resolved by Eden12345

This comment has been minimized.

Sign in to view
@Eden12345

Eden12345 Mar 23, 2020 •
edited
Contributor

It looks like this is the only place that you're using the local state in this component. You can instead just pass the props to the <RadioButton /> component like this:

const { indexClicked, handleItemClick } = this.props
// ...code in between...
	<RadioButton
		checked={index === indexClicked}
		handleClick={() => handleItemClick(index)}
	/>

Then, you can remove all of the code from componentDidMount and RadioButtonGroup.handleClick and not have to worry about local state.

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Mar 31, 2020 •
edited
Author Contributor

Good catch. Just deleted a whole buncha code and even made a functional component
</div>
</div>
))
1 app/scss/panel.scss
View file
Open in desktop
@@ -8,6 +8,7 @@
* https://www.ghostery.com/
*
* Copyright 2019 Ghostery, Inc. All rights reserved.
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0
ProTip! Use n and p to navigate between commits in a pull request.