Skip to content

[dashboard] Update theme preference selector #4021

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

Merged
merged 1 commit into from
Apr 21, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions components/dashboard/src/settings/Preferences.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,34 @@ export default function Preferences() {
<p className="text-base text-gray-500">Choose which IDE you want to use.</p>
<div className="mt-4 space-x-4 flex">
<SelectableCard className="w-36 h-40" title="VS Code" selected={defaultIde === 'code'} onClick={() => actuallySetDefaultIde('code')}>
<div className="flex-grow flex justify-center align-center">
<div className="flex-grow flex justify-center items-center">
<img className="w-16 filter-grayscale" src={vscode}/>
</div>
</SelectableCard>
<SelectableCard className="w-36 h-40" title="Theia" selected={defaultIde === 'theia'} onClick={() => actuallySetDefaultIde('theia')}>
<div className="flex-grow flex justify-center align-center">
<div className="flex-grow flex justify-center items-center">
<img className="w-16 dark:filter-invert" src={theia}/>
</div>
</SelectableCard>
</div>
<h3 className="mt-12">Theme</h3>
<p className="text-base text-gray-500">Light or dark?</p>
<p className="text-base text-gray-500">Early bird or night owl? Choose your side.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: Suggested text can be taken lightly here. If anyone has a better alternative, feel free to update! In any case, looks great for this iteration. 🦉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your text is perfect! 🌟🎯

<div className="mt-4 space-x-4 flex">
<label><input type="radio" name="theme" value="system" checked={theme === 'system'} onChange={() => actuallySetTheme('system')}></input> System</label>
<label><input type="radio" name="theme" value="dark" checked={theme === 'dark'} onChange={() => actuallySetTheme('dark')}></input> Dark</label>
<label><input type="radio" name="theme" value="light" checked={theme === 'light'} onChange={() => actuallySetTheme('light')}></input> Light</label>
<SelectableCard className="w-36 h-32" title="Light" selected={theme === 'light'} onClick={() => actuallySetTheme('light')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Go re-usable components! 🎉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More coming 🔜! 😇

<div className="flex-grow flex justify-center items-end">
<svg className="h-16" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 108 64"><rect width="68" height="40" x="40" fill="#C4C4C4" rx="8"/><rect width="32" height="16" fill="#C4C4C4" rx="8"/><rect width="32" height="16" y="24" fill="#C4C4C4" rx="8"/><rect width="32" height="16" y="48" fill="#C4C4C4" rx="8"/></svg>
</div>
</SelectableCard>
<SelectableCard className="w-36 h-32" title="Dark" selected={theme === 'dark'} onClick={() => actuallySetTheme('dark')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Curious why we named this actuallySetTheme? Isn't setTheme a sufficient alternative? Do we use setTheme elsewhere? 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha 😂 nice catch! If you look above in the code, there is a theme React state (a string), with already a setTheme(value) function.

But if we just call setTheme(value) from the selector component, we'll just switch the React state / selected card, but not the actual web app theme itself.

That's where actuallySetTheme(value) comes in: it calls setTheme(value), but then it also toggles the dark class on the body, and it also updates local storage so that the value is remembered across reloads.

If you see better names for these functions, please share. 😇

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for explaining @jankeromnes! Sounds good! Let's keep this as is. DOES. THE. JOB. 💯

<div className="flex-grow flex justify-center items-end">
<svg className="h-16" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 108 64"><rect width="68" height="40" x="40" fill="#737373" rx="8"/><rect width="32" height="16" fill="#737373" rx="8"/><rect width="32" height="16" y="24" fill="#737373" rx="8"/><rect width="32" height="16" y="48" fill="#737373" rx="8"/></svg>
</div>
</SelectableCard>
<SelectableCard className="w-36 h-32" title="System" selected={theme === 'system'} onClick={() => actuallySetTheme('system')}>
<div className="flex-grow flex justify-center items-end">
<svg className="h-16" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 108 64"><rect width="68" height="40" x="40" fill="#C4C4C4" rx="8"/><path fill="#737373" d="M74.111 3.412A8 8 0 0180.665 0H100a8 8 0 018 8v24a8 8 0 01-8 8H48.5L74.111 3.412z"/><rect width="32" height="16" fill="#C4C4C4" rx="8"/><rect width="32" height="16" y="24" fill="#737373" rx="8"/><rect width="32" height="16" y="48" fill="#C4C4C4" rx="8"/></svg>
</div>
</SelectableCard>
</div>
</PageWithSubMenu>
</div>;
Expand Down