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

Verify button icon format and created updateActionButton() #1134

Merged
merged 8 commits into from Mar 18, 2016
Merged

Verify button icon format and created updateActionButton() #1134

merged 8 commits into from Mar 18, 2016

Conversation

bborgesr
Copy link
Contributor

@bborgesr bborgesr commented Mar 15, 2016

Added a small isIcon() function to make sure that users are using the icon argument (from actionButton and actionLink) correctly (ex: icon=icon("cog")). Otherwise, throw an error.

Created an updateActionButton() function (that applies to both actionButtons and actionLinks) that allows the user to change the input object's label and/or icon.

@bborgesr
Copy link
Contributor Author

A few things:

i) in order to use updateActionButton() to get rid of a previous icon, one should use updateActionButton(icon=character(0)). Winston and I decided this might be the best way to go since NULL is taken for the argument defaults of the update functions. Also, a similar thing is used in other input updates (radioButtons, checkboxGroupInput and selectInput) to clear the list of choices: choices=character(0). (Also, even though there is no reason to use this, and solely for consistency, the actionButton and actionLink functions themselves also do now accept icon=character(0) which has the same effect as omitting the icon argument altogether).

ii) There is also now a validIcon() function that checks that the icon argument passed to any of actionButton, actionLink and updateActionButton is valid. There's 2 possibilities: 1) the icon could be NULL or equal to character(0) or 2) the icon is created through shiny's icon() function. If none of these apply, the app throws a warning. I feel really ambivalent about this. The warning ensures backwards compatibility so that apps don't break if users were using weird (and incorrect) icon arguments. On the other hand, allowing such weird arguments to go through can produce weird and unexpected results... Also, the code looks weird because the function validIcon() returns a boolean, but that isn't actually being used now... So, just throwing an error could be better...

iii) In the Javascript, in order to find any potential previous icon, we look for a child of the button such that 1) it has an i tag and 2) it has some class (including class=""). This prevents italicized text - which has an i tag but (usually) no class from being mistakenly selected.

@bborgesr
Copy link
Contributor Author

Update:
ii) Now throwing an error rather than a warning. If the icon is valid, then validateIcon() (slight name change for consistency with other similar shiny function) returns the icon itself. The code looks much better and no weird results anymore!

bborgesr added a commit that referenced this pull request Mar 18, 2016
Verify button icon format and created updateActionButton()
@bborgesr bborgesr merged commit 1843eca into rstudio:master Mar 18, 2016
@bborgesr bborgesr deleted the updateActionButton branch September 2, 2016 21:23
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.

None yet

1 participant