fix(action-sheet): fix action sheet so it will scroll when the options exceed the screen #13049
Conversation
| @@ -103,13 +106,25 @@ $action-sheet-ios-button-cancel-font-weight: 600 !default; | |||
| @include border-radius($action-sheet-ios-border-radius); | |||
| @include margin(null, null, $action-sheet-ios-group-margin-bottom - 2, null); | |||
|
|
|||
| overflow: hidden; | |||
| overflow: scroll; | |||
There was a problem hiding this comment.
Should we add overflow-y only? So borders do not show on overflow
There was a problem hiding this comment.
- Maybe move this rule to the common
action-sheet.scssfile?
| .action-sheet-ios .action-sheet-group:first-child { | ||
| @include margin($action-sheet-ios-group-margin-top, null, null, null); | ||
|
|
||
| flex-shrink: 2; |
There was a problem hiding this comment.
I think flex-shrink: 1 is enough (or maybe no)
|
|
||
| overflow: hidden; | ||
|
|
||
| flex-shrink: 0; |
There was a problem hiding this comment.
another way to prevent this node to shrink is to use min-height with the correct value (just one idea, not a big deal).
| } | ||
|
|
||
| .action-sheet-md .action-sheet-group:last-child { | ||
| @include padding(0, 0, .8rem, 0); |
There was a problem hiding this comment.
we need also need here: flex-shrink: 0
| } | ||
|
|
||
| .action-sheet-md .action-sheet-group:first-child { | ||
| @include padding(.8rem, 0, 0, 0); |
There was a problem hiding this comment.
add here:
flex-shrink: 2;
MAYBE! we can move this flex-shrink to common css file, so we don't have to duplicate it across all the modes
There was a problem hiding this comment.
- Maybe make the
.8rema variable -
$action-sheet-first-group-padding-top: .8rem;
$action-sheet-last-group-padding-bottom: $action-sheet-first-group-padding-top;
$action-sheet-md-first-group-padding-top: $action-sheet-first-group-padding-top;
$action-sheet-md-last-group-padding-bottom: $action-sheet-last-group-padding-bottom;
$action-sheet-wp-first-group-padding-top: $action-sheet-first-group-padding-top;
$action-sheet-wp-last-group-padding-bottom: $action-sheet-last-group-padding-bottom;- It doesn't seem like there is a need to pad it with
0, agroupdoes not have padding anyway. (usenull)
|
|
||
| background: $action-sheet-wp-background; | ||
| } | ||
|
|
||
| .action-sheet-wp .action-sheet-group:first-child { | ||
| @include padding(.8rem, 0, 0, 0); |
There was a problem hiding this comment.
same here:
flex-shrink: 2;
| } | ||
|
|
||
| .action-sheet-wp .action-sheet-group:last-child { | ||
| @include padding(0, 0, .8rem, 0); |
also adds variables for md and wp mode padding and removes unused $action-sheet-md-group-margin-bottom var
|
Thanks for reviewing @manucorporat and @AmitMY! I just pushed a commit to fix the following:
|
|
love it 😍 , thank u ! |
|
Great work! I'm glad that we can have more than 6 entries! Is there any way to address the "janky-ness" of the scroll now? |
|
@keithdmoore Do you mean the lack of inertia scrolling or is there something else going on? |
|
@brandyscarney I suppose that is what is missing. It just fills really janky. I wanted to allow a user to select a state from a list of all 50 states. When I tried that, it just felt way too janky. I suppose I should create a separate page with a scrollable list and maybe a search bar. Maybe I was just trying to use the action sheet for something it's really not intended to be used for. |
Short description of what this resolves:
Action Sheet will now scroll when the options exceed the screen height, before it would just go off the screen and you couldn't get to them.
Changes proposed in this pull request:
Ionic Version: 1.x / 2.x / 3.x
3.x
Fixes: #12735
Nightly for testing:
[email protected]Before:
After:
Note: need to check the action sheet with the status bar on a device
Gif of all modes:
Gif on an ios simulator: