This repository has been archived by the owner on Sep 5, 2024. It is now read-only.
[WIP] Create DualPaneMapper component and implement step 2 of mapping wizard #40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michaelkro
force-pushed
the
step-2
branch
3 times, most recently
from
1f633d5
to
33ca18c
Compare
|
@michaelkro can you post some screenshots for @serenamarie125 to review after you are getting close? Great job starting this! |
|
@priley86 yup, no problem! |
* Disable add mapping button until a valid mapping has been selected * Remove mapped items from source and target cluster lists after clicking button
* Extract DualPaneMapper as a standalone component * MappingWizardClustersStep will implement instances of DualPaneMapper and TreeView * MappingWizardClustersStep will manage clusters state
michaelkro
changed the title
* Add mappings to TreeView * Only parent nodes are selectable * Parent nodes are expanded by default * Remove individual mappings * Remove all mappings
|
|
|
|
|
|
|
@priley86 @serenamarie125 here are some screenshots. I still have a couple todos (listed in main PR comment) |
michaelkro
force-pushed
the
step-2
branch
2 times, most recently
from
2cd4786
to
7af7b24
Compare
* Create TreeViewContainer component to declutter MappingWizardClustersStep * Fix bug in removeMapping method (missing parentheses) * Add classNames and styles
|
|
|
|
* Optional component that is rendered in the DualPanelMapperList component if `hasCounter` is passed in as true. * Expects `selectedItems` and `totalItems` props
AparnaKarve
reviewed
Feb 13, 2018
| @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; | |||
|
|
|||
| const DualPaneMapperCount = ({ selectedItems, totalItems }) => ( | |||
| <div className="dual-pane-mapper-count"> | |||
| {selectedItems} of {totalItems} items selected | |||
| {selectedItems} {__('of')} {totalItems} {__('items selected')} | |||
There was a problem hiding this comment.
@michaelkro Can you use sprintf here in order to keep the whole sentence intact.
Something like this -
__(sprintf('%s of %s items selected', {selectedItems}, {totalItems}));Whole sentences work better with translators.
serenamarie125
suggested changes
Feb 13, 2018
| {__('Remove Mapping')} | ||
| </Button>{' '} | ||
| <Button disabled={mappings.length === 0} onClick={removeAll}> | ||
| {__('Remove all')} |
There was a problem hiding this comment.
@michaelkro Can you please useInitial capitalization ? "Remove All" with a capital A
There was a problem hiding this comment.
Yup, no problem! :)
DualPaneMapperList The first implementation requires passing props down two levels, first through DualPaneMapperList, then into DualPaneMapperCounter, which is not ideal.
AparnaKarve
reviewed
Feb 13, 2018
|
|
||
| const DualPaneMapperCount = ({ selectedItems, totalItems }) => ( | ||
| <div className="dual-pane-mapper-count"> | ||
| {__(sprintf('%s of %s items selected', selectedItems, totalItems))} |
There was a problem hiding this comment.
Thanks!
Just realized that it should have been -
sprintf(__('%s of %s items selected'), selectedItems, totalItems)
(my apologies)
Otherwise the string would not be extracted for translation.
michaelkro
referenced
this pull request
in michaelkro/manageiq-v2v
Feb 14, 2018
Initial commit based on https://github.com/priley86/miq_v2v_ui_plugin/pull/40
michaelkro
referenced
this pull request
in michaelkro/manageiq-v2v
Feb 14, 2018
Initial commit based on https://github.com/priley86/miq_v2v_ui_plugin/pull/40
|
Closing in favor of https://github.com/priley86/miq_v2v_ui_plugin/pull/44 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[WIP]
Initial implementation and integration of
DualPaneMapperCI failure is due to a
setStatecall incomponentDidMount. Just doing this to stub data for nowhttps://github.com/priley86/miq_v2v_ui_plugin/issues/26
Notes
DualPaneMapper.scsswas copied and pasted from here, with some minor customization.Todo:
x of y items selectedinfo at bottom of Source Clusters pane as shown in mocks