add 404 page for bs4_book and Gitbook #1035

maelle · 2020-12-01T14:12:26Z

A bit hacky
Maybe the message on the page should be customizable

cderv

I think we can do something for gitbook also and close #1046 too.
I'll take it from here but work on your branch in this PR

cderv · 2021-07-01T14:20:57Z

So I went further with my idea with this.

404.html page is now added in the HTML splitting process. This mean it works for gitbook() and bs4_book()
If a 404.html exist in the main repo at the root level with the Rmd files, then it will be used. I don't think this should be used as none of the header, footer and sidebar will be included in this case. This is mainly there to avoid overriding an existing file if it is there. EDIT: then we do nothing and leave it as is. If a user already has a 404.html page then a mechanism is probably in place to move it in output dir (This was changed following a discussion with @apreshill during review)
If a _404.Rmd or a _404.md is there, it will be render as a html_fragment and the content will be included as the body of the 404 page. This means all the theme (navbar, footer, sidebars, ...) will be there.
If none of the above is there, then a default 404.html is created using a simple content (a header, and a body of 2 paragraphs)
Currently, there is no way to opt-in or out of this behavior.

@apreshill before going further with docs and tests, is this something as you imagined ?
We could definitely adapt and simplify.

Demo :

cderv

Some changes to made following live review with @apreshill

Overall we are good with this feature

R/html.R

R/bs4_book.R

cderv

Only thing left is documentation in the book.

Conflicts: NEWS.md

Merge commit 'afadf502bae42334c571da76910e9ca63d59162e' Conflicts: NEWS.md

Conflicts: NEWS.md

cderv · 2021-07-19T13:33:25Z

@yihui Just a friendly ping so that it is on your radar to have a quick thoughts and review from you before deciding to merge this. We can discuss it live tomorrow if you have time to have a look. Thanks!

Behavior of this PR is described here: #1035 (comment)

…w R session; they must be passed explicitly as an argument list

…es like `html_head` to build_404(), and build_404() only creates a raw 404 page, which can be tweaked in split_chapters() later

yihui

@cderv Just two small questions. Thanks!

R/html.R

cderv

Thanks for you changes !

That makes a lot of sense to tweak the function this way. Also existing_files() is a useful function ! 😅 Hard to know them all 😉

I guess you found the idea interesting. That left us with

the choice of what should we do with an existing 404.html: should we copy it (#1035 (comment)) in output dir ?
We need to document this. Where in the book ? Or in which function's help page ? I can do it once we decide.

R/html.R

yihui · 2021-07-20T22:54:04Z

For documentation, perhaps we could add a section in Chapter 6: https://bookdown.org/yihui/bookdown/publishing.html This topic seems to be too small to fit a whole section, though.

cderv · 2021-07-21T08:20:56Z

For documentation, perhaps we could add a section in Chapter 6

This could indeed be a good place to put it in the Github publishing section as 404.html is supported by it. We don't have a Netlify part yet, but it will also be supported. However, no matter where you publish a 404 page will be created though.

I was thinking maybe into:

Chapter 4: https://bookdown.org/yihui/bookdown/customization.html because there will be a default and you can customize it using _404.Rmd. But it is not cross format.
Chapter 3.1: https://bookdown.org/yihui/bookdown/html.html would be a good place too because this is a feature for HTML books only, which will apply on all HTML book format. It could be documented once in Gitbook, and mentioned again in a new bs4_book section with a link back to the Gitbook subsection ? Or a new "3.1.4 Common element" but it seems to short for that.

yihui

We can discuss the documentation later. This PR looks good to me now.

cderv · 2021-07-21T15:39:57Z

I opened a new issue to track the documentation.

And I'll merge this one then.

Thank you !

maelle added 2 commits December 1, 2020 15:09

add 404 page for bs4_book

bfb5f46

fix

26929ae

This comment has been minimized.

Sign in to view

cderv added the bs4_book 🥾 label Mar 31, 2021

cderv added 4 commits July 1, 2021 12:20

Merged upstream/master into maelle-404

0db8cbd

Add 404 page in splitting chapter process

9350609

use explicit name

4694d91

bs4_book can use bookdown feature now

ae1372f

cderv reviewed Jul 1, 2021

View reviewed changes

cderv added 6 commits July 1, 2021 15:13

Adapt text based on @apreshill feedback

f4e70f3

Allow customization using _404.md or providing a 404.html directly

6947d89

Use _404.Rmd or _404.md

567953c

separate in its own function

6013211

build function needs more arguments

2cba367

if 404 is set by md or Rmd, add the edit buttons

53cdb07

cderv changed the title ~~add 404 page for bs4_book~~ add 404 page for bs4_book and Gitbook Jul 1, 2021

cderv linked an issue Jul 1, 2021 that may be closed by this pull request

404 page for gitbook? #1046

Closed

2 tasks

cderv requested changes Jul 1, 2021

View reviewed changes

R/html.R Outdated Show resolved Hide resolved

R/bs4_book.R Show resolved Hide resolved

cderv added 5 commits July 1, 2021 18:54

tweak 404 to remove sidebar

93894e7

Do nothing if 404.html exists already

92c5a72

Add tests for bs4_book

15e9cb9

Start using testthat for new functions

c79badc

Add a NEWS bullet

d95269e

cderv approved these changes Jul 2, 2021

View reviewed changes

cderv added 4 commits July 2, 2021 15:10

Merge commit '28b11a489b843959d5b883208971243a6100b332'

025ffa8

Conflicts: NEWS.md

resolve NEWS conflict

87de9fd

Merge commit 'afadf502bae42334c571da76910e9ca63d59162e' Conflicts: NEWS.md

Merge branch 'upstream/master'

2b0a2e0

Conflicts: NEWS.md

Merged upstream/master into maelle-404

183ed78

cderv requested a review from yihui July 19, 2021 13:33

yihui added 4 commits July 19, 2021 16:16

trivial cosmetic changes

74b7c40

I don't think the variables found and p404 can be found in the ne…

b252098

…w R session; they must be passed explicitly as an argument list

use existing_files()

a1c208f

mostly cosmetic changes so that we don't need to pass several variabl…

33602ad

…es like `html_head` to build_404(), and build_404() only creates a raw 404 page, which can be tweaked in split_chapters() later

yihui approved these changes Jul 19, 2021

View reviewed changes

R/html.R Outdated Show resolved Hide resolved

R/html.R Outdated Show resolved Hide resolved

yihui added 2 commits July 19, 2021 17:43

tweak tests for build_404()

2630883

further tweaking tests

8ac3b62

cderv reviewed Jul 20, 2021

View reviewed changes

R/html.R Outdated Show resolved Hide resolved

don't move the existing 404.html

a527d39

Don't remove empty lines in 404.html created using html_fragment()

a8d98d9

yihui approved these changes Jul 21, 2021

View reviewed changes

cderv mentioned this pull request Jul 21, 2021

Document new 404 page feature #1208

Closed

Bump version

fd73253

cderv merged commit 7203154 into rstudio:master Jul 21, 2021

github-actions bot locked as resolved and limited conversation to collaborators Jan 19, 2022

add 404 page for bs4_book and Gitbook #1035

add 404 page for bs4_book and Gitbook #1035

Uh oh!

Conversation

maelle commented Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

cderv left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cderv commented Jul 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cderv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cderv left a comment

Choose a reason for hiding this comment

Uh oh!

cderv commented Jul 19, 2021

Uh oh!

yihui left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cderv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yihui commented Jul 20, 2021

Uh oh!

cderv commented Jul 21, 2021

Uh oh!

yihui left a comment

Choose a reason for hiding this comment

Uh oh!

cderv commented Jul 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maelle commented Dec 1, 2020 •

edited

Loading

cderv left a comment •

edited

Loading

cderv commented Jul 1, 2021 •

edited

Loading