accompanist icon indicating copy to clipboard operation
accompanist copied to clipboard
verified publisher icon google

[Navigation Material] Add `skipHalfExpanded` parameter with default value equal to `false`.

Open jstarczewski opened this issue 3 years ago • 6 comments

Hey, I added skipHalfExpanded boolean to the parameters of rememberBottomSheetNavigator function, because I think it is a popular use case to make it skip half expanded state as a default behaviour, especially when it is treated as separate navigation destination.

jstarczewski avatar Sep 18 '22 19:09 jstarczewski

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Sep 18 '22 19:09 google-cla[bot]

Hi, thanks for the PR. This is a change we're consciously not making as we rather want to support a destination-level flag. Due to ongoing work in Material's ModalBottomSheetLayout I don't believe we can make this work yet though. Feel free to prototype a destination-level flag, I might be wrong :)

The concerns are about recreating the state in time when the value changes as the destination changes, but maybe it'd work. We'd want some good test coverage that tests

  • Navigating from a skipHalfExpanded = false to a skipHalfExpanded = false destination
  • Navigating from a skipHalfExpanded = false to a skipHalfExpanded = true destination
  • Navigating from a skipHalfExpanded = true to a skipHalfExpanded = false destination and ensures that the sheet opens up to the correct state.

Let me know if you'd be interested in contributing this change!

jossiwolf avatar Sep 19 '22 10:09 jossiwolf

@jossiwolf Alright, on me. I'll try to implement that :)

jstarczewski avatar Sep 19 '22 10:09 jstarczewski

Awesome, thank you!

jossiwolf avatar Sep 20 '22 09:09 jossiwolf

Hey, there hasn't been any progress on this since ~1 year, can I go ahead and create a new pr?

amanv8060 avatar Jul 13 '23 06:07 amanv8060

@amanv8060 Yes go on please, I unfortunetley was not able to find time to adress it properly :(

jstarczewski avatar Jul 13 '23 08:07 jstarczewski

With the release of Compose Material 1.7.0-alpha04, the Material team has added a new artifact: androidx.compose.material:material-navigation, which fully replaces Accompanist Navigation Material.

As such, we are closing all PRs here on Accompanist.

ianhanniballake avatar Mar 08 '24 07:03 ianhanniballake