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.

Sign up for GitHub

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

Jump to bottom

Feature/components set 3 #20

Merged
merged 8 commits into from
Jun 30, 2022
Merged

Feature/components set 3 #20

merged 8 commits into from
Jun 30, 2022

Conversation

filipakkad
Copy link
Contributor

@filipakkad filipakkad commented Jun 22, 2022 •
edited
Loading

PR checklist

  • Add E2E test / update the testing status in the spreadsheet
  • The component has a usage example as a Shiny module
  • The status of the component has been updated in the spreadsheet
  • It has been verified if the component requires a Shiny wrapper, the status updated in the spreadsheet
  • Add a link to the Blueprint documentation

Components:

  • FileInput
  • NumericInput
  • Slider
  • RangeSlider
  • MutliSlider
  • TagInput

Issue for MultiSlider created here

@filipakkad filipakkad force-pushed the feature/components-set-3 branch 2 times, most recently from 8fe43fb to 4d7c54e Compare June 22, 2022 13:43
@filipakkad filipakkad requested a review from kamilzyla June 22, 2022 13:43
@filipakkad filipakkad force-pushed the feature/components-set-3 branch 2 times, most recently from 897a05b to 4dce0c0 Compare June 23, 2022 11:53
@filipakkad filipakkad force-pushed the feature/components-set-3 branch from 4dce0c0 to 591e42b Compare June 24, 2022 10:00
@filipakkad filipakkad force-pushed the feature/components-set-3 branch from 591e42b to df2b51d Compare June 24, 2022 10:11
kamilzyla
kamilzyla requested changes Jun 24, 2022
View reviewed changes
Copy link
Collaborator

@kamilzyla kamilzyla left a comment

Choose a reason for hiding this comment

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

Great job! 👍 My comments are mostly nitpicking; I only have one bigger concern around controlled vs uncontrolled terminology in examples.

inst/examples/components/FileInput.R Show resolved Hide resolved
inst/examples/components/FileInput.R Show resolved Hide resolved
inst/examples/components/NumericInput.R Outdated Show resolved Hide resolved
inst/examples/components/Slider.R Outdated Show resolved Hide resolved
inst/examples/components/MultiSlider.R Show resolved Hide resolved
R/components.R Outdated Show resolved Hide resolved
inst/examples/showcase/app.R Outdated Show resolved Hide resolved
R/components.R Outdated Show resolved Hide resolved
R/components.R Outdated Show resolved Hide resolved
js/src/inputs.jsx Outdated Show resolved Hide resolved
@kamilzyla kamilzyla mentioned this pull request Jun 28, 2022
Merged
5 tasks
@filipakkad
Copy link
Contributor Author

Great, @kamilzyla let me know if we can merge :)

kamilzyla
kamilzyla approved these changes Jun 30, 2022
View reviewed changes
Copy link
Collaborator

@kamilzyla kamilzyla left a comment

Choose a reason for hiding this comment

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

LGTM, that was a lot of work! 🎉 Feel free to merge.

inst/examples/components/RangeSlider.R Outdated Show resolved Hide resolved
@kamilzyla kamilzyla mentioned this pull request Jun 30, 2022
Open
@filipakkad filipakkad merged commit 5ac4694 into main Jun 30, 2022
@filipakkad filipakkad deleted the feature/components-set-3 branch June 30, 2022 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kamilzyla kamilzyla kamilzyla approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@filipakkad @kamilzyla