Open daattali opened 5 months ago
Good question... the difference is that fluidPage
is (still) based on Bootstrap 3.x and we can essentially never change it, nor tons of other UI functions that are built into Shiny, without breaking backwards compatibility. page_fluid
is from bslib and defaults to Bootstrap 5.x, and brings in much better, more modern styles. We think people should generally be defaulting to bslib for new apps nowadays, and our docs are just starting to reflect that. But if you have existing apps based on fluidPage
or other shiny functions, we don't intend to go breaking or removing those in the foreseeable future.
Thanks for the feedback, if you have other thoughts on this "migration" I'd be happy to hear them.
Thanks Joe, that makes sense. I do have some feedback/thoughts to share about bslib vs shiny, where would be the appropriate forum for me to bring them all up? It doesn't have to be a discussion, it can just be a dump of my thoughts and you guys take whatever you want from it.
On a similar note, I wanted to make some comments about the shiny tutorial and articles but couldn't find a github repo associated with it - where do I bring those issues up?
Feel free to put your thoughts here, or as an issue on the rstudio/bslib repo. Also happy to follow up over email or zoom, if needed.
The Shiny for R doc website isn’t currently public but we’re working on opening it up—should happen pretty soon.
OK, since I got the green light from the CTO himself to spam this issue, here goes :) I recognize that some of these may already be on your radar, and some of these are subjective design decisions that you may have already considered and consciously decided against my opinion. You don't have to respond to any of these, I just want to share all my thoughts.
Do all functions in {bslib} require using a page_*()
or bs_theme()
in order to work? When I saw that there was a input_switch()
, I assumed I could just use it in any shiny app (just like with so many other packages that provide extra inputs, the inputs just work without additional setup), but simply including it in an old shiny app will not work and will not give any warning or error. If it's true that everything in {bslib} only works in apps that use page_*()
/bs_theme()
, then the documentation should make that clear
I can get behind {bslib} being a UI package for modern UI, but {bslib} feels like it's going in a few different directions, trying to be too many things. Initially I thought it was just theming. Then I learned that it also includes a lot of layout components, still purely visual. Then I learned that it also includes inputs, which is starting to cross the line for me into a different territory. The package title is "Custom 'Bootstrap' 'Sass' Themes for 'shiny' and 'rmarkdown'" and its GitHub title is "Tools for theming Shiny and R Markdown via Bootstrap 3, 4, or 5". So its stated goal does sound like it was menat to just be a theming package, and evolved to be much more than that. I'd argue it's grown a bit unwieldy. I know it's normal for packages to do more than what their official description is, but there is already so much that {bslib} is doing with theming, that it's enough to warrant a big package on its own. I felt like input_task_button()
belongs in {shiny} because there is nothing theme-y or bootstrap-y about it, and the fact that it's so tightly coupled with shiny's ExtendedTask
makes it more logical for me to see if in {shiny}
It feels a bit messy that there are now two core official shiny packages, made by the same people, that both contain inputs (input_switch()
, input_task_button()
), because from a user perspective there is no different between these and shiny's inputs
A bigger offense to me is that inputs in {bslib} have a vastly different syntax and usage. For example, input_switch()
does not follow any of the same conventions that have been used in shiny for a decade. When I saw a function called input_switch()
, I assumed there would be a update_input_switch()
, but instead it's update_switch()
. The ID parameter for all shiny inputs is inputId
, and the switch decided to change it to simply id
. I understand that if you were to redesign shiny from scratch, these new names might be better, but I don't think you should break existing conventions and cause code to have to mix two different styles of code without a material gain. It's common wisdom in coding that consistency is more important than which style of coding you choose. This is part of the reason why I say that it feels {bslib} and {shiny}, which are not written by a third party but are both by Posit, do not feel like a cohesive unit
Another argument in support for the previous function naming scheme: having the word "input" in the update function name (eg. updateTextInput()
) means that it's clear what the function is doing. For example, suppose you use bslib's naming scheme for shiny's inputs. Then you'd have update_text()
, update_numeric()
, update_date()
instead of updateTextInput()
, updateNumericInput()` etc, which are not at all clear that they are updating inputs
Why are input_switch()
, update_switch()
, toggle_switch()
all grouped together into once doc page and don't contain any info on the different functions? I would find it clearer if each of these functions had its own documentation that clearly states what each function does. The doc is so generic that I don't even know what the difference is between update_switch()
and toggle_switch()
. By the name, I would think that toggle simply changes T -> F and F -> T. But because it takes a parameter, it's not clear to me if that parameter is required? What if it's NULL, then does nothing happen, or does it negate the current value? Is it any different from update_switch()
?
I personally very strongly dislike the current UI implementation of the arrow to collapse/expand the sidebar.The arrow takes a lot of vertical real estate and results in unwanted whitespace, it results in a very unnatural look
I don't love that for there's a recommendation to add class = "bslib-page-dashboard"
to pages, because that's the sort of thing that becomes "hidden knowledge". If there are specific classes that are supported, there should be an explicit list of these classes in an easy to find documentation relating to the page_*
functions
When I was learning shiny, and for a few years afterwards when functions got added, it always felt very streamlined. Everything fit in together very smoothly, very elegantly. It's hard to pinpoint exactly why, but bslib gives me the opposite feeling.
navsets: it's difficult to read through the docs for it because there are so many functions in a single page. It clutters both the namespace and the documentation to have so many functions
navset_tab()
function that encompasses the _tab()
, _pill()
, _underline()
, _hidden()
functions using a parameter. navset_pill_list()
is a different layout and uses unique parameters so it can be its own function. navset_bar()
is entirely different so it should be its own function as well (although it does raise the question of why is there a hamburger-style navbar available yet there is no way to define a navset that automatically changes to a hamburger when on mobile? I think that would be extremely useful to support.) The _card_()
versions of navsets feel like they shouldn't exist - why is there a need to copy several functions, and merely wrap them in a card? I don't see a reason why the _card_()
version has title and sidebar parameters, but the other other navsets cannot. If regular navsets will gain a title and sidebar arguments, then I can simply wrap a navset in a card()
function and it completely removes the need for duplicate functions.nav_select()
, nav_insert()
, nav_remove()
, nav_show()
, nav_hide()
all share the same documentation page. It again causes clutter. If I just want to see the documentation for nav_remove()
, I'm instead bombarded with a lot of functions, and in the argument list I need to figure out which are the arguments that I need vs arguments that used in all the other functions on the page. It would be a much better user experience if looking up a function would only give the info for that function. I can understand that show()
/hide()
might go together, but grouping all of these together feels a bit lazy.
Similar problem with nav_panel()
, nav_menu()
, nav_item()
, nav_spacer()
- they are not the same thing and should not be treated as one. It increases to your cognitive load when you're trying to find the information pertaining to just the one function you care about. Because the actual description of each function is now reduced to being a bullet point inside the "Functions" section of the docs page, you don't get a good description to the function you're interested in.
How does page_navbar()
differ from all the other navsets (eg. navset_tab()
)? I know that traditional shiny also had this concept of a page-level nav vs tabsetPanel, and I always wondered if there's really a need for that distinction, but now that everything is being retohught and reworked, why is this necessary? Can they not be combined? It looks to me like page_navbar()
is very similar to having a navset_underline()
inside a page, with a bit of added padding. Having one dedicated function to make it page-level means that the page-level tabs do not support being pills/tabs, which seems like a very arbitrary decision to make. Likewise, navsets don't have the collapsible
argument that page_navbar()
has, but it would be very useful. I don't see why they can't all be generalized into one.
The shiny tutorial doesn't seem to flow like one unified tutorial, it feels like pieces that were stuck together
lesson1 is a good very high level introduction to shiny, but then lesson2 feels like it's going too deep about all the new components of bslib. Is there really a need to talk about different page_*()
functions and layout_()
functions and value boxes and even throw in {bsicons} right in the beginning? I find it a bit overwhelming and a little bit like a sales pitch for {bslib}. The "Layout Components" tutorial page is a good place that already covers cards and layouts in detail.
lesson2 says at the beginning that they'll learn how to add text, images, and other HTML elements, but those teachings never happen
lesson3: helpText()
is not a control widget, and you should probably avoid mentioning submitButton()
. You should mention somewhere, to lay the ground work for later, that widgets are inputs, to let the user get used to the concept of inputs. The word "input" isn't used at all here, but then when we get to the "Build an App pages", the word "input" is used very often instead of "widget", but a connection between the two is never established
lesson4 says "You can add output to the user interface in the same way that you added HTML elements and widgets." but there was never any mention of how to add html elements or what that even means.
lesson4: don't mention dataTableOutput()
since its officially deprecated
lesson4: There's a table showing the available "output functions" and what each of them creates, and later there's a table showing the available "render functions" and what each of them creates. To reinfornce the point that they must match, the latter table should have another column that shows each of the render functions maps 1-to-1 to the output functions
lesson5 uses choropleth maps, it's a little complex for being in the "Shiny Basics". I would find a simpler sample app to build that helps the user focus on learning shiny rather than trying to learn mapping. It would also be nice if lesson5 and lesson6 built on top of each other, instead of being completely different complex apps. I feel that lesson5 should actually be one of the "case study" build-an-app rather than in the core tutorial
Potential reorganization: I think the "Build an App" pages contain the actual important lessons, whereas I thought they would just be examples to practice. Instead, they are mandatory lessons, and frankly they feel like better "getting started tutorials" than the actual "getting started" pages. The Recap of the first "build an app" is not a recap of the app at all, it just contains new information. It ends with the text "That's all for this module! In the next module we discuss ..." which makes me believe that this entire example really was developed as a standalone shiny tutorial and it was just plugged into here. I feel like the "Shiny Basics" section should be called "Quick Overview" because all the lessons in "Build an App" teach the same information (arguably better) but in more details. Lesson5 is borderline useful, it might make a lot more sense later on as a case study beecause there isn't much there that is really essential to "shiny basics". The "Build an App" section name is very misleading - I would think it means these are case studies and that I already have all the necessary info to make shiny apps, but actually this section provides a lot of lessons, so it should be called "Shiny Basics".
"Contribute" link should mention how we can contribute fixes to the docs website
On the last page of the tutorial there's a big READ NOW button that should link to Hadley's book, but the link is broken
In the Articles page, there's a section for "Structure: Dashboards". Anyone who goes through the tutorial and then lands on this page will have a bad time because most of the UI they were introduced to previously will not work with {shinydashboard}. Either remove that article or make a clear note that {shinydashboard} does not work with {bslib}
In Layout Components, the "Rows" section shows how you can use different row_heights
, but the example shown in the page doesn't have different row heights
Whoa, thanks for the treasure trove of feedback! Lots of actionable stuff for us to go over. Appreciate you taking the time.
For many years the default convention for writing a simple shiny app was
ui <- fluidPage(...)
. I've noticed that in the official docs for layout now mentionpage_fluid()
and don't mentionfluidPage()
https://shiny.posit.co/r/articles/build/layout-guide/I thought that perhaps this means
fluidPage()
is being phased out, so I looked at the function documentation for it and there is no mention there. It seems that any new shiny user will learn to usepage_fluid()
, but as an old shiny user, it's confusing whether I should also be moving to it or not.Documentation should clarify this.