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

feat(components): Export ThemeProvider #434

Merged
merged 35 commits into from
Jan 13, 2024
Merged

feat(components): Export ThemeProvider #434

merged 35 commits into from
Jan 13, 2024

Conversation

nebula-aac
Copy link
Contributor

@nebula-aac nebula-aac commented Dec 13, 2023

UPDATE: I have released an alpha version to test, and the theme provider works as intended in a new app. Using this in Meshery UI will require refactoring, which is what I'm working on.

fix #368

Notes for Reviewers

This PR fixes #

Signed commits

  • Yes, I signed my commits.

fix #368

Signed-off-by: Antonette Caldwell <[email protected]>
add the rest of the components for theme

Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
@nebula-aac nebula-aac added the pr/do-not-merge PRs not ready to be merged label Dec 15, 2023
nebula-aac and others added 9 commits December 15, 2023 14:06
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
@nebula-aac nebula-aac added ready-for-review and removed pr/do-not-merge PRs not ready to be merged pr/draft WIP/Draft pull request labels Dec 19, 2023
@nebula-aac
Copy link
Contributor Author

Ignore the merge conflict, this is only because I release an alpha at v0.13, and I can clean this up when it is approved

hover: darkTeal.dark
};

export const tabeMenu = {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this is an erroneous "e".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eye, Ill make changes

src:
local('QanelasSoftRegular'),
local('Quanelas Soft Regular'),
url('/assets/fonts/QanelasSoftRegular.woff2') format('woff2');
Copy link
Member

Choose a reason for hiding this comment

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

It's nice to see that we're preferring woff2.

function SistentThemeProvider({
children,
emotionCache,
initialMode = 'light'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
initialMode = 'light'
initialMode = 'dark'

return {
fontFamily: ['Qanelas Soft Regular', 'Roboto', 'Helvectica', 'Arial', 'sans-serif'].join(','),
body1: {
fontFamily: ['Open Sans', 'Roboto', 'Helvectica', 'Arial', 'sans-serif'].join(','),
Copy link
Member

Choose a reason for hiding this comment

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

@Rexford74 look! 👀 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm.. nice. Seems the font pairing is being incorporated 🙂

fontFamily: ['Qanelas Soft Regular', 'Roboto', 'Helvectica', 'Arial', 'sans-serif'].join(','),
body1: {
fontFamily: ['Open Sans', 'Roboto', 'Helvectica', 'Arial', 'sans-serif'].join(','),
fontSize: '16px'
Copy link
Member

@leecalcote leecalcote Dec 20, 2023

Choose a reason for hiding this comment

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

@Rexford74 do we have our fonts defined in rem? If so, could you update the list here? If not, could we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's supposed to be rem. I have different theme palettes, so I forgot to add change px to rem here

Copy link
Contributor

@Rexford74 Rexford74 Dec 21, 2023

Choose a reason for hiding this comment

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

Yes, @leecalcote. They are defined in both rem and px. Not only font sizes though.. even line heights are defined in rem and px as well. The font weight for the different sizes was also specified, but I think it is still subject to review.
The idea is not to have too many options to choose from and ensure efficiency in application of the fonts.

Copy link
Member

Choose a reason for hiding this comment

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

rem and only rem is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, actually, including px was for designers using Sistent since rem is not a unit for designing in Figma. I included the rem units specifically because of the engineering end of things to make for seamless handoff.
If I misunderstand your point, please kindly clarify. However, this is the reasoning behind having both units available on Sistent.

@sudhanshutech
Copy link
Member

@nebula-aac when you get time can you quickly toss a doc/readme, i was testing with inline styling but i was getting confused

Signed-off-by: Antonette Caldwell <[email protected]>
@nebula-aac
Copy link
Contributor Author

@sudhanshutech What issues were you having?

…nto 368-themeprovider

Signed-off-by: Antonette Caldwell <[email protected]>
Signed-off-by: Antonette Caldwell <[email protected]>
@nebula-aac nebula-aac merged commit f9b7c8a into master Jan 13, 2024
11 checks passed
@nebula-aac nebula-aac deleted the 368-themeprovider branch January 13, 2024 16:19
@leecalcote
Copy link
Member

Yay!

@leecalcote
Copy link
Member

@senali-d @sudhanshutech fyi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[components]: Export ThemeProvider first
4 participants