Skip to content
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

Updates #199

Merged
merged 16 commits into from
May 16, 2017
Merged

Updates #199

merged 16 commits into from
May 16, 2017

Conversation

bborgesr
Copy link
Contributor

@bborgesr bborgesr commented Mar 23, 2017

The goal is that this will be a commit-sane version of #187

@bborgesr bborgesr force-pushed the barbara/changes branch 4 times, most recently from be2fa71 to 25725a6 Compare April 21, 2017 22:41
@bborgesr
Copy link
Contributor Author

Closes #179 .
Closes #71.
Closes #87.

@bborgesr
Copy link
Contributor Author

bborgesr commented Apr 25, 2017

Need to look better at #114 and #128, to see if these can now be closed as well..

Update: #114 is now closed (with #208 having emerged as a proud sprout) and #128 will be used for future reference (probably when implementing #208). See my loooong note at the bottom of #114 for details.

@bborgesr
Copy link
Contributor Author

bborgesr commented Apr 25, 2017

Closes #38.

NEWS.md Outdated

* Split JS files. (commit [#ea91503](https://github.com/rstudio/shinydashboard/pull/199/commits/ea915038ae2126f48c15e3aac41782a22b16c506)). More updates to Gruntfile and structure. (commit [#4e80616](https://github.com/rstudio/shinydashboard/pull/199/commits/4e80616c5b3aa0dc73022dc815288b5ba7c35be0))

* Added `findAttribute()` function. (commit [#97b952f](https://github.com/rstudio/shinydashboard/pull/199/commits/97b952f13fc5e6781895d49c40e865d4719cb86a))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to mention this, since it's not exported.

NEWS.md Outdated

### New features

* Address [#179](https://github.com/rstudio/shinydashboard/issues/179) support for bookmarking the expanded/collapsed state of the whole sidebar. (commit [#e71c93f](https://github.com/rstudio/shinydashboard/pull/199/commits/e71c93fa7a71f229e725efd4a7867e431cd57679))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For commits, you don't need the #, so instead of #e71c93f, it would be e71c93f.

@@ -3,3 +3,14 @@ This branch of AdminLTE contains the following changes from the stock version, t
* The box collapse function triggers 'shown' and 'hidden' events, so that Shiny knows when outputs are visible or not. See https://github.com/rstudio/shinydashboard/issues/42 for a test app (must re-apply commit 73f6027 when updating to newer version of Admin LTE).

* In AdminLTE.css, the fonts are fetched from the local host, instead of from Google fonts (must re-apply commits e9e63d1 and 9ccb12d when updating to newer version of Admin LTE).

* Add the following code chunk to app.js (see commit #???):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be updated with a ref to the correct commit?

@wch
Copy link
Contributor

wch commented Apr 26, 2017

Other than the small things I commented on, this looks good.

…ed (if any), which makes bookmarking the exact state of the sidebar trivial.

* Better shown/hidden mechanic for Shiny inputs inside collapsible menuItems.
…css transitions look reasonable when its content is initially empty (use case is for hidden Shiny outputs that are not rendered until the first time the menuItem is expanded and reveal them -- i.e. first time that trigger("shown") is called)
# sidebar was collapsed. If this is not the case, the default is whatever the user
# specified in the `collapsed` argument.
dataValue <- shiny::restoreInput(id = "sidebarCollapsed", default = collapsed)
if (disable) dataValue <- TRUE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a comment here to about the transition issue that this is a workaround for?

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