Skip to content

For page_sidebar and page_navbar, consider changing default for fillable to False #938

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

Closed
Tracked by #969
wch opened this issue Dec 22, 2023 · 3 comments · Fixed by #990
Closed
Tracked by #969

For page_sidebar and page_navbar, consider changing default for fillable to False #938

wch opened this issue Dec 22, 2023 · 3 comments · Fixed by #990
Labels
Milestone

Comments

@wch
Copy link
Collaborator

wch commented Dec 22, 2023

For Express mode we had set the default page type to page_fillable(), but changed it back to page_fixed() for reasons explained in #886.

In Express mode, when a sidebar is added, it uses page_sidebar(), which defaults to fillable=True and then runs into the same problem. That means that from the user's perspective, simply adding a sidebar makes a big change in how the main content of the page is laid out, and can lead to surprising errors. For example:

image

Then the user needs to know to use ui.page_opts(fillable=False) to fix it.

Especially in cases where there is a lot of content on the page, I think it would be very surprising for a user that simply adding a sidebar would result in this change; and then in order to understand the fix, they would need to understand the fillable concept, and that the sidebar changes the default behavior of it.

@jcheng5
Copy link
Collaborator

jcheng5 commented Dec 22, 2023

I'm inclined to agree, though I think we should discuss in person with @cpsievert and @gadenbuie as I know they put a lot of thought into it.

@wch
Copy link
Collaborator Author

wch commented Dec 22, 2023

FWIW, I ran into this while working on example apps for the docs, so I don't think it would be an unusual problem to users to encounter.

@wch wch added this to the v0.7.0 milestone Dec 22, 2023
@wch wch added the express label Dec 22, 2023
@cpsievert
Copy link
Collaborator

I can get on board with fillable=False for page_sidebar() -- for consistency, we'll also want that for page_navbar() (and possibly doing the same for bslib?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants