-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
[RNMobile] Update image size UI control #19232
Changes from 2 commits
95e7430
c70193b
82ab9d3
31caac7
c4be661
697ab01
9e71910
9aa8aa7
12ebc85
559fb21
d6be598
34820b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { findIndex } from 'lodash'; | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import Cell from './cell'; | ||
|
||
export default function BottomSheetCyclePickerCell( props ) { | ||
const { | ||
value, | ||
options, | ||
onChangeValue, | ||
...cellProps | ||
} = props; | ||
|
||
const cycleOptionValue = () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A better name for this function imo would be
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might want to handle the case where options is en empty array as well ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
return options[ ( findIndex( options, [ 'slug', value ] ) + 1 ) % options.length ].slug; | ||
}; | ||
|
||
return ( | ||
<Cell | ||
onPress={ () => onChangeValue( cycleOptionValue() ) } | ||
editable={ false } | ||
value={ options[ findIndex( options, [ 'slug', value ] ) ].label } | ||
{ ...cellProps } | ||
/> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import PickerCell from '../mobile/bottom-sheet/picker-cell'; | ||
import CyclePickerCell from '../mobile/bottom-sheet/cycle-picker-cell'; | ||
|
||
function SelectControl( { | ||
help, | ||
|
@@ -17,7 +17,7 @@ function SelectControl( { | |
const id = `inspector-select-control-${ instanceId }`; | ||
|
||
return ( | ||
<PickerCell | ||
<CyclePickerCell | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Though I did not include it in this PR yet. I am proposing that we remove PickerCell.(though Picker is still super useful!). Though PickerCell gets the BottomSheet nested menu about 80% there, it feels like a fundamental restriction of a react-native-modal Modal nested inside of another, that we will not be able to close the containing Modal before showing the child Modal without editing the underlying library. @Tug @etoledom thoughts? One reason this might be worth doing now is in order to decouple PickerCell from Cross Platform component refactoring efforts, like seen here in SelectControl. |
||
label={ label } | ||
hideLabelFromVision={ hideLabelFromVision } | ||
id={ id } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!