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

WiP: Integrate docusaurus 2 #247

Merged
merged 27 commits into from
Nov 19, 2019
Merged

Conversation

wgao19
Copy link
Contributor

@wgao19 wgao19 commented Nov 9, 2019

Picking up from #226, I've adapted most style changes:

  • fixed dark mode
  • aligned both default and code font sizes with existing RSK site
  • code blocks now use this monokai, looks slightly different from the monokai that the current RSK site uses, wondering if @markerikson is ok with this?

requires discussion with @yangshun @endiliey
RSK has a few redirects here, any thoughts how we implement these?

edit: tag the correct yangshun

@endiliey
Copy link

endiliey commented Nov 9, 2019

RSK has a few redirects here, any thoughts how we implement these?

I think its netlify redirect. We can ignore that

I think the netlify preview won't work due to different build folder of v1 vs v2 https://v2.docusaurus.io/docs/migrating-from-v1-to-v2#update-references-to-the-build-directory
A solution is to add netlify.toml to this PR like https://github.com/facebook/create-react-app/blob/master/netlify.toml

@markerikson
Copy link
Collaborator

Erm... those redirects appear to be left over from React-Redux and aren't relevant here :)

@endiliey
Copy link

endiliey commented Nov 9, 2019

It seems that there are some build errros, might need to investigate.

It's currently midnight during my time now and I'm quite occupied this weekend with family's plan. I'll push to Wei's PR maybe in next couple of days.

theme: {
customCss: [
require.resolve('./src/css/custom.css'),
require.resolve('./src/css/monokai.css')
Copy link

@endiliey endiliey Nov 10, 2019

Choose a reason for hiding this comment

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

I think this not a good idea to do this since this means this css will be bundled into main bundle, not when its needed. It will also conflict with the default inline style theme by docusaurus.

The css need to be converted like this https://github.com/FormidableLabs/prism-react-renderer/blob/master/src/themes/palenight.js

and set through “themeConfig.prism.theme”

Copy link
Contributor Author

@wgao19 wgao19 Nov 10, 2019

Choose a reason for hiding this comment

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

i am aware of the converted method although i feel its not completely necessary for the extra work because

  • most if not all pages need this
  • if theres a specificity conflict this theme css should have the lowest specificity to be overwritten by higher specificity selector

Choose a reason for hiding this comment

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

Well I guess its OK because its roughly ~2kb. And the only page that doesn't need it is custom pages.

website/src/pages/404.js Outdated Show resolved Hide resolved
theme: {
customCss: [
require.resolve('./src/css/custom.css'),
require.resolve('./src/css/monokai.css')

Choose a reason for hiding this comment

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

This is the cause of build error >.<

It does not accept array.
https://github.com/facebook/docusaurus/blob/72792a1e5ce19626a591794dc02446749b0193fe/packages/docusaurus-theme-classic/src/index.js#L20

I spent 1 hr + to debug :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • how come the css works then?
  • how to fix this?

Copy link

@endiliey endiliey Nov 11, 2019

Choose a reason for hiding this comment

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

  1. how come the css works then?

It doesn't work perfectly. Its probably some weird and unwanted behavior of webpack or some caching. At least in my side the TOC couldnt appear, and refresh sometimes cause blank page
I've mentioned on Discord that its generating require([])

export default [
  require("infima/dist/css/default/default.css"),
  require(["/mnt/c/Users/endij/Desktop/Linux/redux-starter-kit/my-website/src/css/custom.css","/mnt/c/Users/endij/Desktop/Linux/redux-starter-kit/my-website/src/css/monokai.css"]),
];
  1. HOW TO FIX
    Don't use array. Choose any of this
  • Either convert the css to theme (recommended since 2kbs on home page although small is still unused css)
  • From custom.css import the monokai css
  • Copy monokai css to custom css
  • Put it in static folder, and use stylesheets field in siteconfig
  • Patch docusaurus to support array in theme-classic customCss options
  • Many more ....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol forgive me, updated

Choose a reason for hiding this comment

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

Anyway i fixed it :( its very easy to generate the prism theme.js

Thanks for the hard work

wgao19 and others added 2 commits November 12, 2019 19:04
@netlify
Copy link

netlify bot commented Nov 13, 2019

Deploy preview for redux-starter-kit-docs ready!

Built with commit 56bba80

https://deploy-preview-247--redux-starter-kit-docs.netlify.com

@endiliey
Copy link

endiliey commented Nov 13, 2019

Sorry it took long @markerikson, finally got the time to patch this PR. Was occupied with docusaurus 2 dev itself, including work on versioning.

Anyway, try to take a look at https://deploy-preview-247--redux-starter-kit-docs.netlify.com/
image

@markerikson
Copy link
Collaborator

So, FYI, I just renamed the package from "Redux Starter Kit" to "Redux Toolkit" :) That involved a lot of changes to the content and the site config. All of those will need to be integrated into this branch.

@endiliey
Copy link

Done. It seems that during migration by previous person, there might be something that got missed especially on pages/index.js

@markerikson
Copy link
Collaborator

Yeah. My main concern is with the content of the pages. When I was skimming the PR changes earlier, I thought I saw some conversion of the CodeSandbox embeds from embedded HTML to JSX syntax somehow (and also maybe parts of the doc content too?). Don't want anything to get lost in the translation there.

@yangshun
Copy link

I saw some conversion of the CodeSandbox embeds from embedded HTML to JSX syntax somehow

That's intentional because we use MDX now, so style attributes are props now and have to be objects. The end results are the same though.

I ran the fork locally and made some tweaks. It looks great! Thanks @wgao19 and @endiliey for their efforts in pushing this through!

@markerikson
Copy link
Collaborator

markerikson commented Nov 14, 2019

Skimming through the site, some style-related observations:

  • Quote blocks no longer have a background color. Would be nice to have those stand out more.
  • The left sidebar is moved all the way to the edge of the page, which puts the empty space between the left sidebar and the content. Despite that, it looks like the main content container is smaller - 784px in v1, and 749 in v2. Any reason for that? Also, with the left sidebar on the side, it feels like there's more whitespace overall because the whitespace is next to the content.
  • Related, any chance we can bump up the code block font size a bit? It does appear to be the same-ish as v1, but somehow it feels a bit small overall.
  • The Monokai theme that was dropped in for v2 looks considerably different than the theme for v1. Not the end of the world, but in some cases the highlighting is basically the opposite of v1 in terms of which tokens are highlighted, and which colors.
  • The right-hand TOC current item highlight is bolded in v1, but not in v2, so it's a bit harder to see
  • Something about the shades of gray for the background and the text in inline code snippets feels a bit harder to read in v2
  • Ditto for having links as purple text, instead of blue
  • In v1, inline code snippets in section headers are displayed accordingly in the right-hand TOC (like "Introducing createReducer"), but not in v2
  • Changing the top nav bar from the primary color to the background theme color is quite a big change
  • The v2 mobile theme looks like it takes up a lot less vertical space. A number of folks had complained about having stacked headers fixed at the top of the page with v1.

It does look good overall! Just wondering how many of these can easily be tweaked.

@endiliey
Copy link

endiliey commented Nov 14, 2019

Related, any chance we can bump up the code block font size a bit? It does appear to be the same-ish as v1, but somehow it feels a bit small overall.
The Monokai theme that was dropped in for v2 looks considerably different than the theme for v1. Not the end of the world, but in some cases the highlighting is basically the opposite of v1 in terms of which tokens are highlighted, and which colors.

I’ve tweaked the css again and it should look somewhat similar now. In v1 we use highlightjs but v2 uses prismjs so how they tokenize might be slightly different.

In v1, inline code snippets in section headers are displayed accordingly in the right-hand TOC (like "Introducing createReducer"), but not in v2

Oops that seems like a bug.at first glance. Need to investigate,

As for the rest its very coupled to how infima (a css framework we are using for Docusaurus) so maybe @yangshun can give better explanation on that

@endiliey
Copy link

endiliey commented Nov 14, 2019

In v1, inline code snippets in section headers are displayed accordingly in the right-hand TOC (like "Introducing createReducer"), but not in v2

Fix in facebook/docusaurus#1992. Hopefully next release if got merged

@markerikson
Copy link
Collaborator

Looking better overall. The column width, inline code snippets, and link colors still feel a bit off, but I guess those can be tweaked if we want to.

Anything else you feel needs to be done before this is ready?

@endiliey
Copy link

Not really. Actually stuff like the top nav bar color can be changed. Same case for link colors (but it was intended to use primary colors as link color now). We can always adjust the inline code snippets color as well. Its just CSS.

One thing I'd love to point out is probably the fact that the navigation to page feels almost instant now

@markerikson
Copy link
Collaborator

Yep, definitely feels faster.

Think it's good to merge, then?

@endiliey
Copy link

Hmm i think its ok to merge ? The author of this PR is @wgao19 though, she might want to add something ??

- border radius should be 3px
- bg color should be rgba(27, 31, 35, 0.05)
- anchor color should be blue
- visited color should be redux purple
- outline and link colors should be redux purple
- hover effect flips color and bg
@wgao19
Copy link
Contributor Author

wgao19 commented Nov 19, 2019

I just pushed a few commits addressing part of the styling issues:

  • inline code snippet styles
  • anchor colors
  • pagination button styles

The spacing between left (sidebar) and main content now does look very off to me. This is a cannot unsee situation for me now because the layout looks asymmetrical. Unfortunately I don't have a quick solution. Would like to suggest that we fix the asymmetrical spacing with Docusaurus WDYT @yangshun?

image

Above said, could @markerikson please review the styling changes and merge? And thank you for all the suggestions and support adapting to Docusaurus 2!

@endiliey
Copy link

Just putting my opinion here, By the way,

Ditto for having links as purple text, instead of blue

I think blue won't play well in dark mode and its just weird if the right TOC (which is also a link) is colored purple but the text in docs is blue

image

@wgao19
Copy link
Contributor Author

wgao19 commented Nov 19, 2019

Speaking of accessibility, I feel the primary color doesn't work well with the dark mode neither.

@wgao19
Copy link
Contributor Author

wgao19 commented Nov 19, 2019

I've played around with a few options, all had its own complexities:

  • using a lighter blue for dark mode: none looks well with the primary color
  • using yellow (complementary color for blue) for dark mode: it looks eh
  • using the primary color as link color for dark mode: this behavior is confusing, plus the primary color is also relatively dark for dark mode

Since the current Redux Toolkit site doesn't have dark mode, I feel the most sensible solution is to disable dark mode for now. If we are to enable dark mode in the future, we would have to consider the colors and accessibility issues then.

@markerikson
Copy link
Collaborator

The spacing between left (sidebar) and main content now does look very off to me. This is a cannot unsee situation for me now because the layout looks asymmetrical.

HAH! YOU'RE WELCOME! :)

@markerikson
Copy link
Collaborator

Awright, let's go ahead and merge this in, then. If you have any further style tweaks, please go ahead and file additional PRs.

Think you folks could tackle upgrading the React-Redux docs next? Hopefully that should go a bit faster now that we've got the RTK upgrade as an example.

@markerikson markerikson merged commit c646304 into reduxjs:master Nov 19, 2019
@markerikson
Copy link
Collaborator

Oh, and thank you very much to all of you for your hard work on this! Much appreciated!

@yangshun yangshun deleted the integrate-docusaurus-2 branch November 19, 2019 17:40
@yangshun
Copy link

This is very exciting. Thanks so much Mark!

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.

None yet

5 participants