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

New Gitbook version for the Redux docs - issues, tweaks, and improvements #2846

Closed
BTMPL opened this issue Feb 18, 2018 · 39 comments
Closed
Labels

Comments

@BTMPL
Copy link
Contributor

BTMPL commented Feb 18, 2018

What is the current behavior?

Browser scroll position is not reset when navigating around the new Redux website.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar.

  1. go to https://redux.js.org/advanced/middleware or any other "long" page
  2. navigate to https://redux.js.org/advanced/async-flow or any other "short" page
  3. URL and content is updated, scroll position is not reset and what appears to be "empty" page is shown

What is the expected behavior?

Page is scrolled to top on navigation

CC @SamyPesse as this is most probably a Gitbook issue

@SamyPesse
Copy link
Contributor

Yes, this is a GitBook issue, a fix is already coming (probably tomorrow).

If you catch other issues like this one, feel free to open a ticket at https://gitbook.canny.io/feature-requests

Thank you 🍻

@markerikson
Copy link
Contributor

@SamyPesse : hijacking the thread because you're looking at it. Since we're doing all this redirect stuff anyway - if I wanted to start shuffling locations of some of the docs pages, is there a way to set up redirects?

For example, let's say I wanted to move /advanced/middleware to /intermediate/middleware . Could we get that redirected?

@SamyPesse
Copy link
Contributor

SamyPesse commented Feb 18, 2018

@markerikson GitBook automatically manages redirects. basically if you move a page from hello/a to world/b, then a redirect is setup.

The main issue come from the GitHub integration. When changes are created from pushes to GitHub, it's much harder to recognises moves and not remove & add. GitBook has its own unique identifier for each pages, but it's not currently synced with GitHub.

If you move a page and modify its content at the same time, it'll not be recognized as a move, but as the deletion of the first page and creation of a new page. In that case, no redirect is setup.

My point is that auto-redirects on GitBook works very well for content without GitHub sync, but not really for projects like Redux on GitHub.

You can't currently enter your own redirects, but that may be a nice feature. For example for projects synced with GitHub, we could have an option in .gitbook.yaml:

redirects:
  /advanced/middleware: /intermediate/middleware

But in a perfect world, I'd prefer to have it work automatically for all projects, this is not something docs maintainers should care about; when reordering content, GitBook should be good at avoiding broken urls.

Do you see another usage for redirects ?

@alexkrolick
Copy link

Deprecated/reorganized pages are the main use case for manual redirects. Some of the content may be combined with another page or deprecated while still needing to catch inbound links.

@markerikson
Copy link
Contributor

markerikson commented Feb 19, 2018

Still hijacking the thread, because I don't want to open up a new one for "tweak the Gitbook settings" discussion. In fact, let's just rename this issue.

@SamyPesse , a couple more questions:

  • does the new Gitbook version do docs previews for PRs? If so, is there anything we need to do enable that?
  • Also, the "badges" in the README display poorly in the published docs. Instead of being in a side-by-side row, they're stacked on top of each other and taking up a ton of space: redux-docs-badges Is there a good way we can fix that?

@markerikson markerikson changed the title Redux.js.org - scroll position not reset on page navigation New Gitbook version for the Redux docs - issues, tweaks, and improvements Feb 19, 2018
@SamyPesse
Copy link
Contributor

@BTMPL the scrolling issue has been fixed two days ago 🚀

does the new Gitbook version do docs previews for PRs? If so, is there anything we need to do enable that?

Not yet, but this is planned!

Each commit on master is already linked to a unique "revision" on GitBook with an unique url, for example: https://redux.js.org/~/revisions/-L5nR0C4aT5GpsUn-J59/

screen shot 2018-02-20 at 21 28 46

We plan on extend it to PRs, GitHub will display in the CI statuses a GitBook url for each PR.

Also, the "badges" in the README display poorly in the published docs. Instead of being in a side-by-side row, they're stacked on top of each other and taking up a ton of space

This version doesn't currently support inline images (except in tables). Currently inline images are converted to blocks.

The main reason for not supporting inline images are that it is a very small usage of images, and it's terrible for semantic, edition and layout (sizing is harder to predict).

But one of the usage of inline images is "badges in the readme".

That's why we are investigating multiple solutions:

  • Adding support for inline images (we want to stay as far as possible from this solution because of the reasons I've listed)
  • Stripping inline images from paragraphs already containing other inline images instead of converting to block (it'll strip the badges from the output)

@markerikson Do you think these badges should be visible in the documentation output ?

@markerikson
Copy link
Contributor

I'm totally okay with stripping them out from the docs output. I'd say that's an intended difference between "live README" and "published docs".

@markerikson
Copy link
Contributor

eep, wrong button

@philkunz
Copy link

may I ask what theme this is using?

@timdorr
Copy link
Member

timdorr commented Feb 26, 2018

@philkunz It's Gitbook's new theme for beta. It's custom.

@markerikson
Copy link
Contributor

@timdorr
Copy link
Member

timdorr commented Mar 5, 2018

Interesting bug that @einomi reported: https://redux.js.org/recipes/using-immutable.js-with-redux doesn't work because it's picking up the dot and not returned the right MIME type.

Also, the hash links on the performance page don't work: https://redux.js.org/faq/performance

@SamyPesse
Copy link
Contributor

@timdorr We've released a fix a few hours ago, our CDN was fallbacking some pages to text/plain. You may still have a cache in your browser.

Concerning the hashes, we are working on a fix to correctly parse the <a id="some-hash"></a>. I'll keep you posted as soon as it's live.

@markerikson I'm investigating and we'll publish a fix ASAP.

Did you check out the Search and Insight dashboards ?

For example, it shows that only the search query "Typescript" is popular but doesn't have any result.

@timdorr
Copy link
Member

timdorr commented Mar 5, 2018

You may still have a cache in your browser.

I did, but I don't now. Appears to be fixed on my end! 👍

@markerikson
Copy link
Contributor

@SamyPesse : I'd really appreciate a fix for the remaining redirect issues. We're getting a slew of complaints both here in the repo issues and elsewhere (StackOverflow, etc) that the docs links are broken, and that's really bothering me. It's additional overhead that we don't need to deal with, but more importantly, it's a barrier to people who are trying to just read an external link to the docs.

@SamyPesse
Copy link
Contributor

@markerikson sorry for the delay, it was much trickier issue than anticipated. We've released a fix, but the redirects will be updated for the next update of content done in the GitHub repository.

I'm looking at triggering a reimport of your content to fix it without requiring a commit.

@SamyPesse
Copy link
Contributor

@markerikson
Copy link
Contributor

YAY! THANK YOU! :)

I think we're mostly just missing the manual anchors in the FAQ now, like https://redux.js.org/faq/organizing-state#organizing-state-nested-data .

@markerikson
Copy link
Contributor

@SamyPesse : going back to my earlier comment in this thread, I'd really love to have a way to manually specify redirects.

Now, it sounds like if I were to just rename the "/advanced" folder to "/intermediate" without changing any other content, Gitbook would recognize that (possibly?). But, as @alexkrolick said, we would potentially want to do more shuffling than that, and manual redirects would be a useful tool.

@SamyPesse
Copy link
Contributor

@markerikson I agree, I've discussed it with the team, and we are going to extend the .gitbook.yaml with a redirects option:

#
# Map of urls to redirect to the output of specific files
#
redirects:
  "/hello/world": ./test.md

Will it fit your needs ?

@timdorr
Copy link
Member

timdorr commented Mar 26, 2018

That would be perfect.

@markerikson
Copy link
Contributor

Yup, sounds great!

Going from my admittedly limited knowledge of HTTP here - would that result in an HTTP 301 permanent redirect response?

@SamyPesse
Copy link
Contributor

No, the redirects currently use 302 (Moved Temporarily) HTTP redirects. I think we'll continue that way.

Using 302 (Moved Temporarily) instead of 301 has no impact on SEO, but is cached indefinitely by the browsers.

@markerikson
Copy link
Contributor

Sure, whatever you think works better.

@SamyPesse
Copy link
Contributor

@markerikson @timdorr We've just released the redirects options.

You can extend the .gitbook.yaml with:

redirects:
  some-url/with-a-filename: ./docs/somefile.md

The documentation is at https://betadocs.gitbook.com/integrations/github#configuration but it's still incomplete.

Let me know if you have any feedback, or need help to setup these redirects (ex: I can review a PR before these changes are made on the master)

@markerikson
Copy link
Contributor

AWESOME! Thank you :) I will have to play with this shortly.

One other request that I don't remember if I mentioned before: is PR preview integration in the works?

@SamyPesse
Copy link
Contributor

Yes we plan on adding preview for Pull-Requests.

But probably not this month, we are focusing our efforts on stability, migration from the legacy version, and the launch of the new version (beta.gitbook.com -> www.gitbook.com)

@gaearon
Copy link
Contributor

gaearon commented Apr 19, 2018

I filed an issue about some problems I noticed: #2949

I didn't open the website for a long time and didn't realize it changed so significantly. I'm sorry if my tone is harsh but I'm not very happy with the upgrade and would love to learn more about why we did it.

@gaearon
Copy link
Contributor

gaearon commented Apr 19, 2018

Since we're consolidating the discussion here, I wanted to ask about two issues that seem pressing to me:

  • The badges take a lot of vertical space. This looks bad.
  • There is a new loading state. But it is a static website. It shouldn't need a loading state. It should be able to correctly lay out on the initial load. This loading state creates a bad impression about React and Redux because it matches people's concerns about single-page app UX. We don't want to create that impression.

@SamyPesse Do you expect somebody on the Gitbook team to have time to look into this during the next few days? I totally understand if such issues are not a focus but in this case I think we should move something that matches what we need more closely (e.g. docusaurus which we use on other similar doc websites).

@gaearon
Copy link
Contributor

gaearon commented Apr 19, 2018

To be fair the new version is much more polished in other respects. I think that if we solve these two issues we can stay with Gitbook.

@SamyPesse
Copy link
Contributor

SamyPesse commented Apr 19, 2018

@gaearon Your feedback is totally fair, and I can assure you that the entire GitBook team is working on these points.

Loading and SSR

The loading state is not the expected/normal GitBook experience.

The new GitBook do full server side rendering. But since two days, we had to toggle it off because of an issue that increase our response time.

This is our top priority, the team is working hard on fixing it, as it's not only impacting Redux.

Badges and inline images

As I've explained it, we've previously decided not to support inline images, but instead wanted to implement a workaround for badges.

We've finally started working on a good implementation of inline images. It should be live this weekend.

Let me know if you have more (harsh ;) ) feedback, We want to create the best documentation platform, for Redux, but also for all teams and projects, and feedback always help us !

@gaearon
Copy link
Contributor

gaearon commented Apr 19, 2018

This is awesome, thanks for quick response. Sounds good.

@SamyPesse
Copy link
Contributor

@gaearon Both have been fixed 🚀

The next commit on the docs / README will import the badges correctly.

And SSR is back: https://redux.js.org/ (there may be some caching in your browser)

@timdorr
Copy link
Member

timdorr commented Apr 21, 2018

Just did a tiny commit. The badges look right now 👍

@markerikson
Copy link
Contributor

Cool. I think the last remaining concern is the inline anchor tags for the FAQ section.

@markerikson
Copy link
Contributor

@SamyPesse : fwiw, just saw this comment from a user complaining that the current docs are considerably slower compared to the prior version: https://twitter.com/BraZZeL1ty/status/995296467450519552 .

@Soreine
Copy link

Soreine commented May 24, 2018

@markerikson We are aware that the current version of GitBook is slower on some parts, we are working on it. GitBook docs should be fast, and especially for visitors.


I'm coming to tell you about a potentially breaking change we would like to make to the current docs. Let me explain the situation.

In the current version, the homepage of a doc is not displayed in the Table of Contents on the left. The homepage is accessible when visiting / or when clicking on the doc title in the header. This is confusing because it's not obvious how to access this page from another page.

To remedy this problem, we will simply add the homepage to the Table of Contents on the left. It will look like any other page. The only limitation is you won't be able to add children pages to it, nor to move it under a page group. But this was already an implicit limitation of the previous design.

Here is an exemple of doc, that would change from this:

screen shot 2018-05-24 at 16 21 24

... to this:

screen shot 2018-05-24 at 16 22 36

In your case, this would make your Table of Contents start with a "Read Me" entry. Would you like to adapt your docs before this change is deployed ? Does this change feels right to you ?

@timdorr
Copy link
Member

timdorr commented May 24, 2018

I think that sounds fine to me. Leaving it as "Read Me" sounds correct, since that's the contents of that page and it will set expectations correctly when clicking on it. We don't have a separate intro page for the repo and the docs.

@timdorr
Copy link
Member

timdorr commented May 24, 2018

Also, given there are no pressing issues with the docs at the moment, I'm going to close out this issue. For specific problems, we can open up new issues to track.

@timdorr timdorr closed this as completed May 24, 2018
brandoncroberts pushed a commit to brandoncroberts/redux that referenced this issue Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants