Custom Dropdown component with search support #148
Conversation
|
thanks a lot @AparnaKarve ! i know it is jquery, but it's the most compatible MIQ typeahead right now (as it matches existing MIQ design), and great job wrapping this w/ React. I have linked PF-React #54 to formally revisit this pattern later in PatternFly, but this should work fine for now. I will review it some more first thing tomorrow morning... |
|
hey @AparnaKarve the typeahead functionality is working great here for me... also... tests are still passing 👍 however, when i use the "Continue to Plan" feature on Step 5 Mapping Wizard I am no longer seeing that mapping selected by default in Step 1 of Plan Wizard (seems it is preselecting "Choose" by default). Can you maybe take a look at that one next? |
|
ooOOo this was my browser cache...hard refresh and it working... apologies @AparnaKarve !! this looks good to go... do you want to try the Mapping Wizard selects next in this same PR? |
Sure. Will look into that. |
and use it in Datastores and Networks
AparnaKarve
force-pushed
the
react_bootstrap_select
branch
from
77704b3
to
7263339
Compare
AparnaKarve
force-pushed
the
react_bootstrap_select
branch
from
7263339
to
3d32a22
Compare
|
@priley86 Can you review again when you get a chance? I have added some new screenshots above for Datastores and Network screens where this component is now being used. |
There was a problem hiding this comment.
@AparnaKarve this looks really nice! Thanks for doing this!
| return ( | ||
| <div> | ||
| <SourceClusterSelect |
There was a problem hiding this comment.
Can we delete the SourceClusterSelect component now @michaelkro @AparnaKarve ?
There was a problem hiding this comment.
Fo sho! This is a huge upgrade, nice job @AparnaKarve!
There was a problem hiding this comment.
👍 alright sounds good. If we remove SourceClusterSelect I think this PR is good to merge 😄
There was a problem hiding this comment.
Thanks for the feedback guys -- will be deleting SourceClusterSelect now...
|
nice job!! |
| } | ||
| choose_text={`<${__('Select a source cluster')}>`} | ||
| render_within_form="true" | ||
| form_name={form} |
There was a problem hiding this comment.
So, since we're now hooking the selects into redux-form, this enables us to make a couple improvements
- Lift the
selectedClustercomponent state to the redux store - We can now use
redux-formvalidation to handle thenextbutton disabling. Before I had to use a workaround where I was renderingDualPaneMapper, but hiding it (find hack here). We can instead uselengthfromredux-form-validatorsand go back to conditionally rendering this guy
If we want to make these changes, I imagine it would be best to do it as a separate PR
There was a problem hiding this comment.
yes - i think lifting selectedCluster to redux store is fine (so good w/ # 1).
For # 2 (handle next button disabling)... i think that's probably OK but we have to vet this change w/ @mturley 's wizard abstraction. I believe shouldDisableNextStep(activeStepIndex) callback is called for a step now, so we'd just have to verify we can still map this to redux-form-validators. This should all jive in a future PR... but I'm fine w/ what's been added here...
This component is a wrapper for
bootstrap-selectand is a good substitute for react-bootstrap-select(react-bootstrap-select is unusable because of reasons mentioned here - https://github.com/priley86/miq_v2v_ui_plugin/pull/140#issuecomment-372491569)
Component used in Datastores step -
Component used in Networks step -