.collapse('hide') on hidden uninit-ed collapsible no longer shows it #16011

cvrebert · 2015-03-06T11:17:25Z

Fixes #15315.
Closes #15807.
X-Ref: #14282, #14417

CC: @XhmikosR @hnrch02 @juthilo for review

cvrebert · 2015-03-06T11:35:42Z

Damn. The one old test this makes fail dates back to the advent of the Collapse plugin's test suite, so we might need to ask @fat about this.
I'm not sure why it's testing for adding style="height: 0px;", considering that .collapse without .in sets display: none anyway...

hnrch02 · 2015-03-06T13:43:05Z

js/tests/unit/collapse.js

+    setTimeout(done, 0)
+  })
+
+  QUnit.test('should hide a collapse when initialized with "hide" if not already hidden', function (assert) {


Maybe change the name to "should hide a collapse when initialized with "hide" option if not already hidden"

I'm basically just following how the analogous "show" tests above these are phrased

hnrch02 · 2015-03-06T13:46:52Z

I think it's save to remove/rework that old test.

@peterblazejewicz

…ixes #15315 Thanks to @peterblazejewicz & @Nikita240 Adds unit tests based on #14417 X-Ref: #14282 Closes #15807

Appears to be unnecessary and is broken by #16011, which is otherwise quite sane.

cvrebert · 2015-03-06T15:07:26Z

Ok, took the plunge and removed that one old assertion.

Appears to be unnecessary and is broken by #16011, which is otherwise quite sane.

cvrebert · 2015-03-08T11:58:38Z

@XhmikosR LGTY?

XhmikosR · 2015-03-09T08:02:09Z

Makes sense. 👍

.collapse('hide') on hidden uninit-ed collapsible no longer shows it

Fixes a Bootstrap bug introduced by twbs/bootstrap#16011

… scenarios by patching a BS3 bug introduced twbs/bootstrap#16011

…e navbar scenarios by patching a BS3 bug introduced twbs/bootstrap#16011" This reverts commit 4ba02c9.

@schloerke

… 1.5.0 Ahmed Mohamed (1): Fix debounce() behavior when r() throws an error Alan Dipert (116): Add MockShinySession$makeScope() (#2714) .testModule(): remove args parameter fixes #2709 (#2713) Add shiny::markdown() for inline Markdown markdown(): improve docs, simplify call to markdown_html(), default to extensions = TRUE Type stability changes to shinytestrun object add_result() for test run add_result() for test run add_result() for test run Make improvements suggested by @schloerke runTests() output Add working runTests test revert names in app1 tests More progress on runTests() format Pass existing runTests() tests Normalize runTests() output, improve documentation of returned dataframe Fix some roxygen errors Remove redundant new test NEWS.md updates Add noRd to private function Remove Rd Subtle change to .testModule() semantics Fix global reference test inside testServer Minor changes and tests Fix some roxygen errors New session$env test Add test to mitigate shadow with unquote Change test to not be locale-dependent Address feedback markdown(): Improve docs; add .noWS; class result as html minor reformat tests Update modules.R Add S3 class to MockShinySession Add back many working/converted tests Desired environment semantics are working Split up and rename various tests More test reorg Bring back scope tests More test progress test-module.R => test-server.R First passing app dir test for testServer overhaul appobj coercion works Doc and test updates document document document Add failing proxy-related and ns() related tests WIP more markdown() tests WIP mock session scoped proxy nested module tests pass now, many others fail %-) fix one failing test Getting there Tests pass \o/ getEnv() => env, docs Document new R6 methods document Inline markdown tests, add to docs Add markdown() NEWS item Use vapply in mapNames() Remove unused variable Un-inline assignment Move session$env sanity check out of makeMask and into testServer Use base versions of a couple rlang::env_* functions Clarify testServer lexenv assertions Remove strings from expect_error server => module Bump glue dep NEWS.md Consolidate testServer() fixes and enhancements (#2815) testServer() and loadSupport(): if app is a path, and not an ap… (#2823) loadSupport(): fix global.R support, run global.R in appropriat… (#2831) Dynamic dots for MockShinySession$setInputs() (#2838) testServer(): Properly capture module return values testServer(): Users can now pass in their own session add mock method notify func A good test of real/mock session difference Use noop() WIP noop refactor party; collation brokn roxygen flailing Add more noops and errors Add instance methods in the constructor fix tests Clarify test logic; better output Fix ls() usage, revert MockShinySession roxygen tags Down to 1 missing method and 1 missing field roxygen tweaks Remove unnecessary constructor Un-noop a couple things we need to implement registerDownload() and private$renderFile Update private$downloads docs getCurrentOutputInfo() downloadHandler() test downloadHandler() test getCurrentOutputInfo() works, and tests Test reactive() + downloadHandler Return invisible in noop functions ensyms => list sapply => names() <- + lapply <<- to <- export MockShinySession; document roxygenation roxygenation sp renderCachedPlot() support, minor reorg and misc. fixes get_mocked_publics() cleanup Pass namespaced names to renderFile() Call session ended callbacks, fix related existing/broken test Ensure temp files passed to downloadHandler() content functions are deleted on session close Fix tempdir()-related problem Fix pkgdown test testServer() doc update Fix new CMD check note get() => !!as.symbol() Add reactive/promise test Move callModule() to separate .Rd; add various links to docs (#2889) Drop testModule() from NEWS.md, fixes #2901 Alex Gold (1): Add label to file reload observer Barret Schloerke (102): display auto reload base directory for reactlog label Use a frozen lock file within CI Bust appveyor cache every time the DESCRIPTION file updates add install cmd Add comment about why we are busting the cache Add more comments about why we are busting the appveyor cache Fix broken timer tests and check htmltools docs (#2758) Invoke onSessionEnded callbacks with self reactive domain bump version add news item update js files (version bump) Remove slack notifications make sure the server is only run if the example is interactive get travis to pass for now un-skip test Update roxygen level Fix Travis checks (#2791) Add class `'function'` to `reactive` and `reactiveVal` objects (#2793) add examples for actionButton and actionLink which adds extra css classes Add `./revdep` folder output and script (#2790) Use roxygen2 reexport setup for htmltools and fastmap (#2795) Update Node.js build scripts (#2800) Update actionButton example (#2806) add type = 'hidden' for tabsetPanel to hide the tab headers document add news items. Add tabPanelBody() function. Document Add `updateActionLink` (#2811) use a list. Use @cpsievert 's wording suggestion. add !important attr to .nav-hidden css class roxygen sorted the reexports Merge error / result column as `pass` can be used to determine what the value is pass tests Add todo Reprint error add print method Add assert logic Each testing environment must require their own loadSupport call if necessary add another testing app that has a module that returns a reactive value Remove code specifically looking for shinytest only files. This is not necessary as it will still work in the current setup. Add an environment argument to `runTests` white space and small comments Document Edit docs Rename output Enable broken adhoc test Update failure name Add contexts and pass all tests Update app to be consistent. Add testthat contexts Commit revert Throw error on legacy shinytest testing instead of allowing the legacy test structure Update to use withr / loadSupport Update tests with the design that shinytest legacy can not be called. Add `assert=FALSE` where appropriate Code feedback. snake case legacy_shinytest fn. add comments / change error print the app folder name (if available), not just the test file name No longer need helper-support. testthat.R should do this before running use adhoc.R in stead of server.R in shinyAppTemplate Add full template + runTests test Remove runTests output feedback - Do not require req(input$size) feedback - drop adhoc feedback - no `expr = ` Have output$sequence test be conditional on if the r dir is used Copy all files, but if they are glue files... use the template feedback - remove all non-module/server test files in the testthat dir if the R folder is not used If no module is used with shinytest, do not test the module shiny.autoload.r is not required for runTests anymore Test many combinations of shinyAppTempalte combos. Do not full matrix as shinytest is slow to execute Document Test module app 107_scatterplot. Use pretty paths when printing the runTests output removed dplyr remove autoload.r test, as that code path doesn't exist anymore display the test folder name when it fails fix test use mkdir_p to always create directories add comments make sure shinytest is installed from github. Needs latest version safeguard testing by checking for some suggested packages merge news items code feedback Do not flush when setting the returned value in a mocked shiny session Do not set a returned value for an app replace ... with args in testServer remove '../' from loadSupport calls as they will be found automatically now use list instead of rlang::list2 testServer should return invisible() do not call monitorHandle unless it is set check if function, not if not null Move 'value' to first arg position for tabPanelBody. 'value' is now required. Add tests code feedback for tabPanelBody use 'id' instead of 'value' for tabPanel/tabPanelBody testServer does not return results. Do not test for it. remove duplicate param docs Update tests/testthat/test-tabPanel.R Update tests/testthat/test-tabPanel.R Remove check note about function not found Remove trailing comma and extra return statement feedback from @wch. Keep 'value' arg, drop 'id' undo id parameter for example Use GitHub Actions (#2876) Run pre-revdepcheck for v1.5.0. Found 2 errors. Made 2 PRs (#2885) bump version to 1.5.0. Build JS run revdep. updates redep_cran notes Carson (79): wip bs4 compat work remove debugger; more detailed comment dateRangeInput() should use input-group-sm, not input-sm or form-control-sm btn-outline-secondary is visually closer to btn-default (compared to btn-secondary) progress-bar-danger class has been replaced by bg-danger progress-bar-striped needs to appear on the actual progress bar (not it's container) Compatibility for checkboxes will happen via bs4 shims For some reason event.relatedTarget isn't populated when the previously active item is a dropdown item showcase compatibility Need to de-active any nav tab, not just navs in a dropdown requires htmltools >= 0.4.0.9001 undo unnecessary diff; yarn build document with modern roxygen New path tag causes scoping issue in showcase mode document Bump jquery 3.x to 3.5.0, closes #2856 Use native String.trim() method since $.trim() is now deprecated yarn build news Have getCurrentOutputInfo() return NULL instead of error if called in a non-reactive context (extension of #2707) update news wip grep for hex codes properly and return exit length 0 character string if input is length 0 Support a named vector as an override to computed colors only assign default for aes that are relevant and non-transparent facet strips shouldn't have stroke and fill should be a semi-lighten fg color Add support for lattice and better default palette() for col scaling Report font-family and colors in getCurrentOutputInfo() if .shiny-report-theme class is present make sure ggplot_apply_auto_colors always returns a built plot Add accent (link) color and qualitative color palettes Add sequential colorscale for ggplot2 based on the accent color code review feedback; introduce autoThemeOptions() mix colors using scales::colour_ramp Set default scales via plot_env for old ggplot2 and options for new ggplot2 cleanup allow autoTheme options to be reactive Add autoThemeOptions() to pkgdown suggest scales; other R CMD check things default to bg='white', not transparent newpage should always come before ggplot_build sequential colorscale now mixes fg/bg with accent (for the endpoints) port auto-theming logic to new thematic package wip font support First pass at an auto-theming interface Auto-theming interface will come from thematic ragg will take priority over Cairo User-supplied bg to renderPlot should take 1st priority no need to call thematic Provide a device argument to renderPlot() and plotPNG() code review with Barret bump version missed passing device in renderCachedPlot() somehow messed up rebase change device bg default only if the thematic option is set Auto values aren't resolved until plot time, so if we see one, resolve it Rollback the custom device arg (may come later) in favor of a shiny.useragg option Pass check let showtext know about the resolution default to FALSE for now code review with Winston generalize internal is_installed and use it in startPNG() comments Use is_available() more widely and remove unneeded complexity in check_suggested() Wrap styles in reactive() so that calling getCurrentOutputInfo() doesn't always invalidate update news and add to comment smaller bump in version make shiny.useragg an unofficial option that takes priority over quartz safe-guard against NA values Use getStyle() to support old browsers Explain the return value of getCurrentOutputInfo() roxygen Output context isn't well-defined shiny.useragg should default to FALSE remove renderedFamily info field Fixes #2896 by avoiding fatal JS exception in some collapsible navbar scenarios by patching a BS3 bug introduced twbs/bootstrap#16011 bump jQuery 3.5.0 -> 3.5.1 Revert "Fixes #2896 by avoiding fatal JS exception in some collapsible navbar scenarios by patching a BS3 bug introduced twbs/bootstrap#16011" Finish updating to jQuery 3.5.1, add a tools script, add documentation to tools/README.md news update Dean Attali (1): typo in NEWS Hadley Wickham (8): Combine documentation for textOutput() and verbatimTextOutput() Improve inputFile() accept documentation Tweak shinyApp docs about bookmarking Update docs Don't set default id in clickOpts() and friends Remove deprecated arguments from plotOutput/imageOuput Combine docs for nearPoints() and brushedPoints() Rebuild docs Jeff Allen (4): Don't notify slack on success Integration Testing Docs (#2691) Expand testServer/Module docs (#2694) [WIP] Add skeleton function and example (#2704) Joe Cheng (23): wip1 wip2 Remove unnecessary bs4 classes Remove unnecessary bs4 compat shims from showcase Remove btn-outline-secondary Remove branch from htmltools remote Get rid of striped progress bar style Remove .col-form-label No need for bg-danger, progress-bar-danger is now in bs3compat Support shiny.autoreload even when there are errors Revert "Support shiny.autoreload even when there are errors" Support shiny.autoreload even when there are errors Respect shiny.autoreload option being set in app.R or global.R Implement plot.autocolors for cached plots More robust un-setting of autocolor params More autocolor fixes Tweak ggplot2 colors to look closer to defaults Better autocolors for bars and other geoms that don't have colour; lighter lattice defaults Use htmltools::parseCssColors Safer default file deletion behavior for renderImage Unit tests Code review feedback isTempPath is now isTemp, simplified semantics (#2886) JooYoung Seo (1): Fixed an incorrect line in tools/README.md for IonRangeSlider Mine Çetinkaya-Rundel (2): Minor updates to shinyAppTemplate docs Remove reference to helper-load.R that's no longer there Winston Chang (56): Document insertUI() and removeUI() together Run Travis on more R versions Relax test so it passes on R 3.3 and below Preserve matrix dimensions in dataTablesJSON. Fixes #2653 Bump version to 1.4.0.9000 Update NEWS Make shiny.autoload.r default to TRUE (#2659) Update NEWS Datepicker: make disabled months and years lighter gray Update NEWS Make cursor indicate when dates are not selectable Update to roxygen2 7.0.0 Update documentation of Progress and MockShinySesssion getCurrentOutputInfo(): return NULL if not in an output Update docs Rebuild shiny.js Rebuild docs with roxygen2 7.0.1 Bump node version on Travis Rebuild docs with roxygen2 7.0.2 Use as.raw instead of charToRaw Timer functions always use ms, and better names. Closes #2725 Use test_path Fix NEWS item Add moduleServer function Add NEWS item Skip debounce/throttle tests on CRAN Add link Change indenting of module examples Don't print loading R/ dir messages (#2817) Update NEWS Fix typo shinyAppTemplate tweaks (#2828) Small fixes Add migrateLegacyShinytest function Update test for new function name Documentation edits Fix pkgdown.yml Clearer wording for dryrun option Add sort_c function for sorting in C locale Make runnable example for downloadButton App template tweaks Fix link Ensure file encoding is specified for sort_c Clearer language for shinyAppTemplate prompt Add updateFontAwesome script Update Font-Awesome to 5.13.0 Update NEWS Rebuild docs Update README Update tools/README.md Update NEWS Remove old manual test app. Closes #2892 Refer to shinytest migration function Remove links from NEWS.md Remove Remotes field Rebuild docs with dev version of roxygen ginberg (1): replace devtools by remotes trestletech (49): Adding a test runner Add namespace rewire option. Add a filter to test Load the helpers before the tests -- into an ancestor environment of the tests'. Update contributing instructions for cla-assistant. Best-effort task scheduling through the session Revert "Best-effort task scheduling through the session" Best-effort task scheduling through the session Add mock timer class Unrelated: fix autoload tests. Attempt to get the current time from the session, if available. Consolidate to two Timer classes and simplify conditionals PR feedback. Broke tests because of dependency on session, though, so might revert. Revert "PR feedback. Broke tests because of dependency on session, though, so might revert." Bring back the good parts of a003 Introduce MockShinySession Include equality in scheduling comparisons Try to fix failing Windows test Align default parameter values with real session PR feedback Rename parameter, destroy old outputs Bring in promise helpers and test for getOutput auto-flushing Export flushReact method Refine which methods merit warnings Simplify run_now() Introduce integration testing functionality Handle Joe's feedback. Test improvements for Windows and make CHECK pass. Cleaning up the vignette Add message about loading R/ Disable autoloading with a R/_disable_autoload.R file. Change URL Pin to the beginning of the file path. Clean up test warnings Implement missing test. Export session Placeholder docs for MockShinySession How do I still get paid to do this? PR feedback Regenerate docs Remove vignette. Rename testApp -> runTests Add entry in pkgdown Load shiny and inherit from global env. Non-error are no NA and errors are passed through. Set working dir in testServer so the app runs in the proper dir. PR feedback Don't use GH deps for appveyor Attempt to pin roxygen2 Add a message to clarify what app is being tested, support setting progress on mock session.

cvrebert added the js label Mar 6, 2015

cvrebert added this to the v3.3.4 milestone Mar 6, 2015

hnrch02 reviewed Mar 6, 2015
View reviewed changes

.collapse('hide') on hidden uninit-ed collapsible no longer shows it; f…

872e56f

…ixes #15315 Thanks to @peterblazejewicz & @Nikita240 Adds unit tests based on #14417 X-Ref: #14282 Closes #15807

cvrebert force-pushed the fix-15315 branch from e880137 to 872e56f Compare March 6, 2015 14:58

cvrebert added a commit that referenced this pull request Mar 6, 2015

Collapse unit test: rm check that no-op hide adds style="height: 0px"

3a17e91

Appears to be unnecessary and is broken by #16011, which is otherwise quite sane.

cvrebert mentioned this pull request Mar 6, 2015

Fix #15354, #15315: Uninitialized collapsibles should not toggle on .collapse('hide') #15807

Closed

Collapse unit test: rm check that no-op hide adds style="height: 0px"

5c8d56d

Appears to be unnecessary and is broken by #16011, which is otherwise quite sane.

cvrebert force-pushed the fix-15315 branch from 3a17e91 to 5c8d56d Compare March 7, 2015 02:11

cvrebert added a commit that referenced this pull request Mar 9, 2015

Merge pull request #16011 from twbs/fix-15315

21d5637

.collapse('hide') on hidden uninit-ed collapsible no longer shows it

cvrebert merged commit 21d5637 into master Mar 9, 2015

cvrebert deleted the fix-15315 branch March 9, 2015 08:03

cvrebert mentioned this pull request Mar 9, 2015

v3.3.4 ship list #15624

Closed

cpsievert referenced this pull request in rstudio/shiny May 15, 2020

Don't assume option is always a string, closes #2896

a077f40

cpsievert added a commit to rstudio/shiny that referenced this pull request May 15, 2020

Don't assume option is always a string, closes #2896

9a9686e

Fixes a Bootstrap bug introduced by twbs/bootstrap#16011

cpsievert added a commit to rstudio/shiny that referenced this pull request May 15, 2020

Fixes #2896 by avoiding fatal JS exception in some collapsible navbar…

4ba02c9

… scenarios by patching a BS3 bug introduced twbs/bootstrap#16011

cpsievert added a commit to rstudio/shiny that referenced this pull request May 18, 2020

Revert "Fixes #2896 by avoiding fatal JS exception in some collapsibl…

948244b

…e navbar scenarios by patching a BS3 bug introduced twbs/bootstrap#16011" This reverts commit 4ba02c9.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

.collapse('hide') on hidden uninit-ed collapsible no longer shows it #16011

.collapse('hide') on hidden uninit-ed collapsible no longer shows it #16011

cvrebert commented Mar 6, 2015

cvrebert commented Mar 6, 2015

hnrch02 Mar 6, 2015

cvrebert Mar 6, 2015

hnrch02 Mar 6, 2015

cvrebert Mar 6, 2015

hnrch02 commented Mar 6, 2015

cvrebert commented Mar 6, 2015

cvrebert commented Mar 8, 2015

XhmikosR commented Mar 9, 2015

.collapse('hide') on hidden uninit-ed collapsible no longer shows it #16011

.collapse('hide') on hidden uninit-ed collapsible no longer shows it #16011

Conversation

cvrebert commented Mar 6, 2015

cvrebert commented Mar 6, 2015

hnrch02 Mar 6, 2015

Choose a reason for hiding this comment

cvrebert Mar 6, 2015

Choose a reason for hiding this comment

hnrch02 Mar 6, 2015

Choose a reason for hiding this comment

cvrebert Mar 6, 2015

Choose a reason for hiding this comment

hnrch02 commented Mar 6, 2015

cvrebert commented Mar 6, 2015

cvrebert commented Mar 8, 2015

XhmikosR commented Mar 9, 2015