fix(ui-containers): Use display: contents

[gadenbuie]

Allows children of conditionPanel() and uiOutput() to automatically participate in the grid/flex layout of the parent container.

For uiOutput() we use div:where(.shiny-html-output), for two reasons:

1. This only applies when inline = FALSE or a div() container is used.
2. The :where() ensures that users can use .shiny-html-output without fighting our rule.

For posit-dev/py-shiny#881

[jcheng5]

Can you think of any scenarios this might break existing apps?

[gadenbuie]

First and most obvious: this would break apps where users were directly styling the conditionalPanel() or uiOutput() container using selectors that target our the emitted markup directly. It's hard to know in advance how those apps would be broken.

In most other cases, I believe setting display: contents is much closer to what users are expecting happens with the children of the conditional panel or UI outputs. In particular that it's much closer to adding the children directly into the layout.

The original issue in posit-dev/py-shiny#881 is a good example that it's not obvious that both approaches add intermediate containers. Outside of flex/grid layouts the impact of the layout container is minimal, but there's greater impact now that we're using both flex and grid more often.

[martinamorris]

It turns out our shiny application statnetWeb is one of those that breaks with this change.

As the current maintainer, and someone not well versed in this syntax, can you pls tell me if there is a straightforward change to the 2 example code chunks below that will update to the new shiny syntax required:

conditionalPanel(condition = 'input.filetype < 5',
               column(6,
                    br(),
                    fileInput(inputId='rawdatafile', label=NULL, accept='text'),
                    verbatimTextOutput('rawdatafile'))
                )

             conditionalPanel(condition='input.filetype == 3',
                 column(6,
                        uiOutput('pajchooser'))),

Many thanks!

[gadenbuie]

Hi @martinamorris, could you please open a new issue? It'd be helpful if you could describe a bit more how the app has broken and how we could reproduce the issue. It's possible that your issue is related to the change in this PR, but it could also be something else -- this PR didn't involve any syntax changes to conditionalPanel().

[martinamorris]

Will do, thx. The break occurred with the 1.8.1 CRAN update, and affects the page rendering for pages constructed with conditionalPanel. I followed the link from your 1.8.1 release tag to this commit, but this may not be the culprit.

Will show how I reproduced the issue, with some screenshots.

[martinamorris]

done: #4099

[ismirsehregal]

With this change merged, I can no longer use this workaround, can I?

[cpsievert]

@ismirsehregal from what I can tell, the workaround offered in that issue still works? Are you encountering a different issue?

[gadenbuie]

@cpsievert I came to the same conclusion -- but while looking at the reprex I started think we should initially hide the contents of a conditionalPanel() until it's been initialized.

[ismirsehregal]

@cpsievert thanks for the feedback. I just read the NEWS:

This is probably what most users intend when they use these functions, but it may break apps that applied styles directly to the container elements created by these two functions.

and thought it might affect the above mentioned "workaround" which I'm still using every now and then.