Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Reduce complexity and 'black-boxed' nature of tab panel logic - #3372

#3372
Merged
cpsievert merged 13 commits into
masterrstudio/shiny:masterfrom
li-dropdownrstudio/shiny:li-dropdownCopy head branch name to clipboard
May 5, 2021
Merged

Reduce complexity and 'black-boxed' nature of tab panel logic#3372
cpsievert merged 13 commits into
masterrstudio/shiny:masterfrom
li-dropdownrstudio/shiny:li-dropdownCopy head branch name to clipboard

Conversation

@cpsievert

Copy link
Copy Markdown
Collaborator

(To be merged after rstudio/htmltools#215)

In #3315, a lot of complexity was added to buildTabset() (and the JS behind insertTab(), etc) due to this Bootstrap 4+ regression (at the time, I thought it was a "known" regression, but apparently it was an oversight). That regression has already been fixed in {bslib}'s version = 4 (via rstudio/bslib#306), hence the bump in {bslib}'s version requirement.

Comment thread R/bootstrap.R Outdated
@cpsievert
cpsievert marked this pull request as ready for review April 27, 2021 16:47
Comment thread R/bootstrap.R Outdated
@wch

wch commented May 5, 2021

Copy link
Copy Markdown
Collaborator

I believe that tagQuery() should preserve attribute order. Can you provide a minimal example where it doesn't?

@cpsievert

Copy link
Copy Markdown
Collaborator Author

I believe that tagQuery() should preserve attribute order

You're right. I checked with Barret and it should preserve HTML attribute order (it doesn't preserve ordering of the $name, $attribs, and $children elements, though)

I was getting confused by the fact that, in the old behavior, active vs non-active tabs will have different ordering of their attributes. If you think it's important we maintain that (inconsistent) ordering, I can do it, but it'll introduce more complexity

@cpsievert

This comment has been minimized.

@cpsievert
cpsievert merged commit 3e33755 into master May 5, 2021
@cpsievert
cpsievert deleted the li-dropdown branch May 5, 2021 15:05
cpsievert added a commit that referenced this pull request May 10, 2021
schloerke added a commit that referenced this pull request May 17, 2021
* master: (48 commits)
  Modify `icon()` function to call `fontawesome::fa_i()` for equivalent functionality (#3302)
  Follow up to #3372: fix oversight in refactor (#3387)
  Revert "Do not double pull within rituals"
  Do not double pull within rituals
  Remove deprecated code and parameters (#3137)
  Prettify TS
  Rebuild JS files
  Add important flag
  Update comments
  Add sendImageSize2
  Use removeSheet()
  Make more CSS properties !important
  Add note about synchronous behavior in IE
  Rebuild JS files
  Simplify IE CSS handling
  New strategy for sending information when CSS loads
  Make sure dev version of rlang is available (#3382)
  Reduce complexity and 'black-boxed' nature of tab panel logic (#3372)
  Install dev version of rlang (#3379)
  Comment about the hoisting
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.