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(rich-text-editor): fix dark mode ui appearance #696

Merged
merged 13 commits into from
Jun 29, 2022

Conversation

htuerker
Copy link
Contributor

Fixes #537

Screen.Recording.2022-06-28.at.03.13.29.mov

This is a very minimal solution. I tried something else but it seemed overkill then left it like that.

@notsidney This gives at least a chance to make a theme switch without issues.

For future references on this issue, there might be a solution something like the below:

const useDisabledStylesheets(mode, sheet, cache) {
  useEffect(()=> {
    if(!cache[mode]) {
        cache[mode] = sheet;
    }
    sheet.disabled = false;
     return () => {
      cache[sheet].disabled = true;
    }
  })
}

import "/foo.css"
const LightCSS = (cache) => {
   useDisabledStylesheets("light", document.stylesheets[document.stylesheets.length - 1]), cache]
   return null;
}

@notsidney
Copy link
Contributor

I noticed that when switching from light to dark, it correctly shows the dark menu. But when switching from dark to light, it’s stuck on the dark menu (the background and text colors are both white).

This is because importing the CSS file results in a <style> tag with the CSS to be appended to the <head>, but doesn’t remove any previously imported styles that have been unmounted. So the result is:

<style> light styles </style>
<style> dark styles </style>

Then the dark styles will always take precedence from now on.


This can be solved by writing the <style> tag ourselves:

// Editor styles
/* eslint import/no-webpack-loader-syntax: off */
import skinCss from "!!raw-loader!tinymce/skins/ui/oxide/skin.min.css";
import skinDarkCss from "!!raw-loader!tinymce/skins/ui/oxide-dark/skin.min.css";

...

<style>{theme.palette.mode === "dark" ? skinDarkCss : skinCss}</style>

The second problem is that the iframe used to display the content doesn’t re-render when the theme changes. The <Editor> component receives an updated content_style prop, but doesn’t update the iframe itself. We can force this to happen by setting key={theme.palette.mode} on the <Editor> component itself (which remounts the entire component).

This is ok to do because the user’s input is stored in state in the component containing <RichTextEditor>, so when the editor is remounted, we don’t lose the user input.

I’ll write review comments on specific lines, which you can approve yourself, then I’ll merge this PR 👍

Copy link
Contributor

@notsidney notsidney left a comment

Choose a reason for hiding this comment

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

Please commit the suggestions. Feel free to expand the <GlobalStyles> to further improve styling to match our theme.

src/components/RichTextEditor.tsx Outdated Show resolved Hide resolved
src/components/RichTextEditor.tsx Show resolved Hide resolved
src/components/RichTextEditor.tsx Outdated Show resolved Hide resolved
src/components/RichTextEditor.tsx Outdated Show resolved Hide resolved
src/components/RichTextEditor.tsx Outdated Show resolved Hide resolved
src/components/RichTextEditor.tsx Outdated Show resolved Hide resolved
src/components/RichTextEditor.tsx Outdated Show resolved Hide resolved
src/theme/RichTextEditorDarkCSS.tsx Outdated Show resolved Hide resolved
src/theme/RichTextEditorLightCSS.tsx Outdated Show resolved Hide resolved
@htuerker
Copy link
Contributor Author

This can be solved by writing the <style> tag ourselves:

I disagree. This can only be solved: scan all the entire appended stylesheets then disable invalid ones by reverse engineering. Just joking 😄 Thanks for your guidance, by the way!

@htuerker htuerker marked this pull request as draft June 28, 2022 18:02
htuerker and others added 11 commits June 28, 2022 21:04
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>
@htuerker
Copy link
Contributor Author

I think this is good to go! @notsidney

Screen.Recording.2022-06-28.at.23.03.06.mov

Copy link
Contributor

@notsidney notsidney 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 to me!

Also just wondering why you’re importing GlobalStyles from tss-react instead of MUI—is there an advantage there that I’m not aware of?

Oh, I just realised I forgot to put that suggestion in. Either should be fine since we’re already using tss-react elsewhere, because of the migration from MUI v4 to v5 away from JSS.

@notsidney notsidney marked this pull request as ready for review June 29, 2022 02:31
@notsidney notsidney linked an issue Jun 29, 2022 that may be closed by this pull request
@notsidney notsidney merged commit 28c1d67 into rowyio:develop Jun 29, 2022
notsidney added a commit that referenced this pull request Aug 10, 2022
* Bump ejs from 3.1.6 to 3.1.8

Bumps [ejs](https://github.com/mde/ejs) from 3.1.6 to 3.1.8.
- [Release notes](https://github.com/mde/ejs/releases)
- [Changelog](https://github.com/mde/ejs/blob/main/CHANGELOG.md)
- [Commits](mde/ejs@v3.1.6...v3.1.8)

---
updated-dependencies:
- dependency-name: ejs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump minimist from 1.2.5 to 1.2.6

Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6.
- [Release notes](https://github.com/substack/minimist/releases)
- [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6)

---
updated-dependencies:
- dependency-name: minimist
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump tmpl from 1.0.4 to 1.0.5

Bumps [tmpl](https://github.com/daaku/nodejs-tmpl) from 1.0.4 to 1.0.5.
- [Release notes](https://github.com/daaku/nodejs-tmpl/releases)
- [Commits](https://github.com/daaku/nodejs-tmpl/commits/v1.0.5)

---
updated-dependencies:
- dependency-name: tmpl
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump protobufjs from 6.11.2 to 6.11.3

Bumps [protobufjs](https://github.com/protobufjs/protobuf.js) from 6.11.2 to 6.11.3.
- [Release notes](https://github.com/protobufjs/protobuf.js/releases)
- [Changelog](https://github.com/protobufjs/protobuf.js/blob/v6.11.3/CHANGELOG.md)
- [Commits](protobufjs/protobuf.js@v6.11.2...v6.11.3)

---
updated-dependencies:
- dependency-name: protobufjs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix(rich-text-editor): fix dark mode ui appearance (#696)

* fix(rich-text-editor): fix dark mode ui appearance

* Update src/components/RichTextEditor.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* Update src/components/RichTextEditor.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* Update src/components/RichTextEditor.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* Update src/components/RichTextEditor.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* Update src/components/RichTextEditor.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* Update src/components/RichTextEditor.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* Update src/components/RichTextEditor.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* Update src/theme/RichTextEditorDarkCSS.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* Update src/theme/RichTextEditorLightCSS.tsx

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* fix(rich-text-editor): add stylings to dropdown

* fix(rich-text-editor): add toolbar stylings

* fix(rich-text-editor): reset hover&focus bg

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* update date & time filter operators for clarity

* Action field: prevent selecting self as required field (fixes ROWY-551)

* Date & Time: only show date for date filters

* move fullScreenButton to be shared, remove md settings

* bundle-analyzer

* Leaf icon: use mdi-material-ui

* Feat: Percentage field color customization (#692)

* feat(percentage-c11n): convert to table cell

* feat(percentage-c11n): add logic to default configs

* feat(percentage-c11n): add color picker to settings

* feat(percentage-c11n): change default colors

* feat(percentage-c11n): fix button text color

* feat(percentage-c11n): add labels to settings

* feat(percentage-c11n): add preview section

* feat(percentage-c11n): fix cache issues with debouncing

* feat(percentage-c11n): add width responsiveness to color picker

* feat(percentage-c11n): fix responsiveness issues

* feat(percentage-c11n): add checkbox, refactor a little

* feat(percentage-c11n): convert data type to array

* feat(percentage-c11n): refactor config states

* feat(percentage-c11n): fix defaults

* feat(percentage-c11n): add basic cell without bg

* feat(percentage-c11n): remove collapse

* feat(percentage-c11n): refactor checkStates

* feat(percentage-c11n): add grid layout

* feat(percentage-c11n): chore conventions

* feat(percentage-c11n): add default theme color to sidedrawer

* remove redundant fragment

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* fix text color in preview

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* fix: change state to derived state

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* fix: review suggestions

* fix: remove redundant change call

Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* fix(percentage-c11n): remove redundant dependencies

Co-authored-by: Shams <shams.mosowi@gmail.com>
Co-authored-by: Sidney Alcantara <sidney@sidney.me>

* extend callable timeout to over 9minutes

* fix timeout value

* fix page loading with white screen while system is in dark mode

* Revert "bundle-analyzer"

This reverts commit dd214b9.

* fix nav items not accessible with Tab

* Percentage: don’t display if value null or undefined

* fix NavDrawer causing compile to fail

* show text field if collections array is empy

* column ids

* row ID

* fix create table showing empty dropdown for collections

* fix row not writing to db once all required fields are written

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Han Tuerker <46192266+htuerker@users.noreply.github.com>
Co-authored-by: shamsmosowi <shams.mosowi@gmail.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.

Bug in RichText field Headers option
2 participants