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

Initialize Chakra Theme #100

Merged
merged 5 commits into from
Feb 4, 2023
Merged

Initialize Chakra Theme #100

merged 5 commits into from
Feb 4, 2023

Conversation

Myrfion
Copy link
Contributor

@Myrfion Myrfion commented Feb 3, 2023

What I did?

I created the folder theme where we gonna have all theme-related files. For now, it has index.ts which imports all other files and initializes the theme itself. Another file is colors.ts where I added the Seneca colours in spectra from 100 to 900 and that colour I called brand (this is how chakra does it in the example in the doc so probably this is the way to go in terms of naming). I also created the button on index.ts just to show how the component that uses the theme looks like

How to test?

Just run the app locally and check the front end, you should see the button with very familiar color

@Myrfion Myrfion requested review from Eakam1007 and humphd February 3, 2023 02:42
@humphd
Copy link
Contributor

humphd commented Feb 3, 2023

@Myrfion can you please rebase on main to get rid of the conflict you have, and also post a screenshot?

@Myrfion
Copy link
Contributor Author

Myrfion commented Feb 3, 2023

Log in screen (button has a brand color scheme):

image

Log out screen (button has a brand color scheme):

image

@humphd here it is

humphd
humphd previously approved these changes Feb 3, 2023
import { extendTheme } from '@chakra-ui/react';
import colors from './colors';

const theme = extendTheme({ colors });
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we set the default colorTheme prop for Buttons, etc. as well (i.e., so you don't have to remember to do it each time)? See chakra-ui/chakra-ui#2840

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this would be really helpful

Copy link
Contributor

Choose a reason for hiding this comment

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

We can do it in a follow-up, @Myrfion/@Ririio if one of you wants to file it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can implement the button default theme right now. We do the others later on

@SerpentBytes
Copy link
Contributor

@Myrfion I was able to test it locally, and it works. Looks good to me. 👍🏼

@SerpentBytes SerpentBytes self-requested a review February 3, 2023 15:44
SerpentBytes
SerpentBytes previously approved these changes Feb 3, 2023
Eakam1007
Eakam1007 previously approved these changes Feb 3, 2023
Copy link
Contributor

@Eakam1007 Eakam1007 left a comment

Choose a reason for hiding this comment

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

Could look into setting a default color theme for different components, but looks good

@Myrfion Myrfion dismissed stale reviews from Eakam1007, SerpentBytes, and humphd via bc4db98 February 3, 2023 19:06
app/theme/components.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@Eakam1007 Eakam1007 left a comment

Choose a reason for hiding this comment

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

Looks good

@Myrfion Myrfion requested a review from humphd February 3, 2023 22:31
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

This is great

Comment on lines +41 to 43
<ChakraProvider theme={theme}>
<Outlet />
</ChakraProvider>
Copy link
Contributor

Choose a reason for hiding this comment

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

we can use ChakraBaseProvider to only focus the theme on components that we actually want to have a different color like, button, box, grids, etc. The website is not big so it's not a big deal, but it's just a suggestion

import { extendTheme } from '@chakra-ui/react';
import colors from './colors';

const theme = extendTheme({ colors });
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can implement the button default theme right now. We do the others later on

app/theme/components.ts Outdated Show resolved Hide resolved
app/theme/components.ts Outdated Show resolved Hide resolved
@Myrfion Myrfion merged commit de5336c into main Feb 4, 2023
@Myrfion Myrfion deleted the feature/34/theme-definition branch February 4, 2023 00:32
@humphd humphd mentioned this pull request Feb 4, 2023
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.

5 participants