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

fix(component): Check for window in components #124

Merged
merged 3 commits into from
May 17, 2022

Conversation

tulup-conner
Copy link
Collaborator

SSR apps do not define `document` or `window` on
initial render, since they are *hydrated* by React
thereafter.

We need to make sure any usages of `document` or
`window` by the library depend on the following
condition first:

```js
windowExists() && ...
```

In the event something must be returned, even if
`window` is `undefined`, return `undefined`. This
temporary state will be overriden once hydrated.
@tulup-conner
Copy link
Collaborator Author

I tested the Modal and Flowbite wrappers on my own Remix installation with success, so AFAIK, this should resolve the issues with Next.

@tulup-conner tulup-conner marked this pull request as ready for review May 17, 2022 00:19
@rluders rluders merged commit 4b3b8c5 into themesberg:main May 17, 2022
@tulup-conner tulup-conner deleted the fix/check-document-for-ssr-apps branch May 17, 2022 05:28
tulup-conner added a commit to tulup-conner/flowbite-react that referenced this pull request Feb 20, 2023
So, I originally fixed this issue across every component in themesberg#124, but the bug was reintroduced by

resolve themesberg#609
tulup-conner added a commit to tulup-conner/flowbite-react that referenced this pull request Feb 20, 2023
So, I originally fixed this issue across every component in themesberg#124, but the bug was reintroduced by

resolve themesberg#609
tulup-conner added a commit to tulup-conner/flowbite-react that referenced this pull request Feb 20, 2023
So, I originally fixed this issue across every component in themesberg#124, but the bug was reintroduced by

resolve themesberg#609
rluders pushed a commit that referenced this pull request Feb 20, 2023
* feat(theme): adding theme prop to components

* feat(/theme/default): finish migrating components to use a `root` theme

Follow up to #500

BREAKING CHANGE: Like in #500, this version permanently changes the `FlowbiteTheme` for numerous
components.

The philosophy is that themes will more clearly reflect the component's structure.

For example, an `<Accordion>` can contain any number of `<Accordion.Title>` or `<Accordion.Content>`
sections. The theme used to look like:

```js
accordion: {
  base: "..",
  content: "..",
  flush: "..",
  title: "..",
}
```

And now, the theme for an `<Accordion>` looks like:

```
js
accordion: {
  root: {
    base: "..",
    flush: "..",
  },
  content: "..",
  title: "..",
}
```

So now the options in the theme which apply to the `<Accordion>` itself will always be found under
`root`. Likewise, `<Accordion.Content>` can be themed via the `content` subsection.

This ultimately will apply to all components.

* ci(eslint): remove `prettier` plugins for `eslint`

Instead, use `prettier-plugin-tailwindcss`, which is sufficient.

* refactor(/lib/*): use `yarn prettier` with `prettier-plugin-tailwindcss`

* fix(/lib/components/*.spec): resolve test errors caused by migrating theme

* feat(/lib/components/*): add `theme={}` attribute to components that need it

See notes in #566

* fix(/lib/components/accordion): fix `<Accordion theme={}` types wrong interface

* docs(/pages/theme): update `/theme` documentation to include new theme strategies

We have more powerful options to customize themes now, and they deserve proper documentation.

* ci(eslint): fix `eslint` configured to ignore `src/lib/`

Well this sucks! We've not been linting the vast majority of the actual code of the library due to a
misconfigured `.eslintignore`. Mass-fix coming.

* ci(eslint): resolve outstanding `eslint` issues

* fix(/lib/components/footer/footertitle): allow `<Footer.Title as="..">` to not use `<h2>`

You can cast the component to a component of your own, or a generic HTML element, e.g.,
`<Footer.Title as="h3">`.

resolve #594

* fix(/lib/components/modal): fix `Modal` expects `document` to exist

So, I originally fixed this issue across every component in #124, but the bug was reintroduced by

resolve #609

* ci(.github/workflows/build): upgrade `codecov-action` -> `v3`

---------

Co-authored-by: Ricardo Lüders <rluders@redhat.com>
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.

2 participants