Select component: add prop multiple
#102
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
==========================================
+ Coverage 89.36% 89.39% +0.03%
==========================================
Files 84 84
Lines 1438 1443 +5
Branches 213 217 +4
==========================================
+ Hits 1285 1290 +5
Misses 124 124
Partials 29 29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
forman
left a comment
forman
left a comment
There was a problem hiding this comment.
Cool, thanks!
- Please add an entry to
CHANGES.md. - Always form Python code using "black" (or "ruff") and for TS use "prettier"
There was a problem hiding this comment.
- do not format entire
CHANGES.mdfiles only because you added a new item - avoid updating
package-lock.jsonwhenpackage.jsonhasn't changed
EDIT
In addition:
- the formatting of TypeScript files uses a wrong style, use
prettier Select.tsxcoverage dropped by ~5%, please add a test case that covers themultipleproperty
b-yogesh
left a comment
b-yogesh
left a comment
There was a problem hiding this comment.
Overall it looks good. Please have a look at my comments and change requests. Thanks :)
| ext.add(my_panel_2) | ||
| ext.add(my_panel_3) | ||
| ext.add(my_panel_4) | ||
| ext.add(my_panel_5) |
There was a problem hiding this comment.
I would prefer if we limit the number of demo's here, as there is already some backlog with a few more demos. It would be good, if we can add your demo to Panel C as it already has a Select element, and you can now show that we can use multiple elements by updating the text accordingly (which is also very similar to your demo).
There was a problem hiding this comment.
I'd prefer, if no longer continue that way. In another pull request we should rearrange the demo app entirely.
Co-authored-by: b-yogesh <yogesh09121995@gmail.com>
…multiple # Conflicts: # chartlets.js/CHANGES.md # chartlets.py/CHANGES.md # chartlets.py/demo/my_extension/__init__.py # chartlets.py/demo/my_extension/my_panel_5.py
This PR closes #101.
This PR:
multipleto theSelectComponent