geist-org / geist-ui

A design system for building modern websites and applications.
https://geist-ui.dev
MIT License
4.35k stars 335 forks source link

Custom theme is broken in latest release #459

Closed pankaj9296 closed 3 years ago

pankaj9296 commented 3 years ago

Bug report 🐞

The latest version of GeistUI not supporting theme customization. When I set the theme to "light" and set a custom palette, it doesn't reflect on the site. Code: https://github.com/pankaj9296/create-next-app/blob/master/pages/_app.js Vercel deployment: https://create-next-app-olive.vercel.app/

Also when I set the theme to "Custom", it builds the project successfully but throws an error on page load "An unexpected error has occurred." Code: https://github.com/pankaj9296/create-next-app/blob/develop/pages/_app.js Vercel deployment: https://create-next-app-ae613lkyu.vercel.app/

Also, the Grid and other components are broken in the latest release: Check my live site, it's all broken: https://iconvey-app-5bxc74haf.vercel.app/ It used to look like this: https://iconvey-app-2wq17a38v.vercel.app/

I had to revert back to use the 2.0.3 version to fix it temporarily.

Version & Environment

Expection

The behavior I expect is, it should not break on page load and the theme should reflect on components.

Actual results (or Errors)

I got an error: An unexpected error has occurred.

code
ofekashery commented 3 years ago

Hi @pankaj9296, first, thanks for the demos, I was able to reproduce it easily.

When you create a Theme, you should to use Themes.create / Themes.createFromLight / Themes.createFromDark to import all the default values into your theme. Note that the light / dark names are in use, so the first example will not work.

When you are missing values in the Theme (like gap and breakpoints), the Grid will not work either, like in the third example.

This is how it should look like:

const myTheme = Themes.createFromDark({
  type: 'my-light-theme',
  palette: {
    success: '#6C63EE', // primary/theme color
    link: '#6C63EE',
    successLight: '#7b73ff',
    successDark: '#574fdb'
  }
});
pankaj9296 commented 3 years ago

Thanks @ofekashery for the quick response. The custom theme is now working.

Although I've one more issue with the latest version. the GRID is broken. Attached a video that shows the issue. the new CSS property "display: inherit" seems to be breaking all existing GRID for me. https://user-images.githubusercontent.com/17493609/108215513-6d03ef80-7157-11eb-9c1c-2f87cdcc9a59.mov

Please let me know if I'm doing anything wrong or you need a demo deployed on Vercel.

unix commented 3 years ago

@pankaj9296 display: inherit; is the correct behavior, and inherit means that in most scenarios, the default property of display is flex, so, alignItems and justify such flex proprietary properties to work properly.

If your layout is broken, don't worry, these solutions will help you: (just choose one of them)

<Grid xs={24} justify="center" alignItems ="center" direction="column" />
<Grid><div style={{ width: '100%' }}></div></Grid>
<Grid xs={24} style={{ display: 'block' }} />

@ofekashery Although display: inherit will automatically set display to flex, but I think it would be better to just set it to flex, what do you think about here? The non-flex mode is really rare and does not need to be considered separately, if the user really needs non-flex mode, directly with the style override on the good.

pankaj9296 commented 3 years ago

Thanks @unix for the help. I'll go with the second solution to temporarily fix all GRID items for now.

pankaj9296 commented 3 years ago

Hi guys,

I just found another issue with the latest version of geist-ui.

On the page load, the theme takes some time to apply on page load. it creates a minor blink. Like on page load the Success color is blue and after a moment, the color changes to purple (my theme color). here's my latest preview deployment if you want to review https://iconvey-app-lc9hnnik7-pankaj9296.vercel.app

FYI, I'm using tree-shaking to reduce bundle size, I guess that doesn't affect it.

Thanks

unix commented 3 years ago

I guess this may be related to server-side rendering, which will take some time to debug, I will follow up on this issue.

pankaj9296 commented 3 years ago

Great Thanks, Witt.

unix commented 3 years ago

I think it's related to this problem (#258), but still no best solution. If anyone would like to share their best practices, please reopen it.