Snapping #16

RareScrap · 2021-11-07T12:47:36Z

Issue #14

Extracted defaults

+ Extracted defaults

onebone · 2021-11-14T07:08:02Z

@RareScrap I have scanned your code so far, I have some possible options that can deal with the difficulties you have pointed at #14.

For scroll strategies other than ExitUntilCollapsed, they need to modify offsetY which is a member of CollapsingToolbarScaffoldState. The cause derives from the fact that snap behavior is actually a feature of CollapsingToolbarScaffold. I expect the problem would be fixed if you move the snapping methods to ScaffoldState.

And for interrupting animation with user inputs, I think it may be solved by utilizing MutatePriority though it may need some extra researches.

lib/src/main/java/me/onebone/toolbar/CollapsingToolbar.kt

RareScrap · 2021-11-25T05:31:52Z

Thank you for your review. I was little busy last weekend. Will done with PR at this weekend.

onebone · 2021-11-25T11:50:14Z

Great!

…rScaffoldState + Defaults naming refactoring + TODOs added

RareScrap · 2021-12-01T06:07:14Z

There are many TODOs that require your attension. Please comment on each of them so we can discuss them.

onebone · 2021-12-02T08:37:58Z

Thank you for reminding, I will review it until this weekend!

lib/src/main/java/me/onebone/toolbar/CollapsingToolbarScaffold.kt

onebone · 2021-12-05T16:14:45Z

lib/src/main/java/me/onebone/toolbar/CollapsingToolbarScaffold.kt

+		}
+	}
+
+	// TODO: Is there a better solution rather OptIn ExperimentalToolbarApi?


What makes it bother you??

I can't decide what to use: @ExperimentalToolbarApi or @OptIn(ExperimentalToolbarApi::class). I suppose that @ExperimentalToolbarApi is using for public animation methods and @OptIn(ExperimentalToolbarApi::class) safe to use in internal methods but I'm not sure.

What are you think? Why you using @ExperimentalToolbarApi at all?

Expierence

Sorry, I don't understand you. What exactly do you mean by this?

lib/src/main/java/me/onebone/toolbar/SnapStrategy.kt

lib/src/main/java/me/onebone/toolbar/CollapsingToolbarScaffold.kt

- when toolbar is expanded or collapsed fully, do not perform snap (as it is not needed), this is the case, when progress == 0 or 1 - this improves performance and fixes a bug with horizontal scrolling in the CollapsingToolbarScaffold content

Add check whether snap to perform snap

ChristopherKlammt · 2022-01-17T12:21:59Z

@onebone what are the plans for merging this PR?

RareScrap · 2022-01-17T16:07:24Z

There is a bug in it. I will describe it tomorrow.

Marx6

Coded

Marx6 · 2022-01-19T08:59:35Z

lib/src/main/java/me/onebone/toolbar/CollapsingToolbarScaffold.kt

+		}
+	}
+
+	// TODO: Is there a better solution rather OptIn ExperimentalToolbarApi?


onebone · 2022-01-19T13:32:15Z

lib/src/main/java/me/onebone/toolbar/ScrollStrategy.kt

+}
+
+// TODO: Is there a better solution rather OptIn ExperimentalToolbarApi?
+@OptIn(ExperimentalToolbarApi::class)


expandOffset and collapseOffset are experimental because it is subject to change their behavior or surface anytime. Meantime, performSnap is private and we don't have to be warned by this potential change so I think it would be sufficient.

onebone · 2022-01-19T13:33:40Z

lib/src/main/java/me/onebone/toolbar/AppBarContainer.kt

@@ -76,15 +67,15 @@ fun AppBarContainer(
 	modifier: Modifier = Modifier,
 	scrollStrategy: ScrollStrategy,
 	/** The state of a connected collapsing toolbar */
-	collapsingToolbarState: CollapsingToolbarState,
+	collapsingToolbarScaffoldState: CollapsingToolbarScaffoldState,
 	content: @Composable AppbarContainerScope.() -> Unit
 ) {


Please revert contract(i.e. parameter or behavior) changes in AppBarContainer.kt as it is in maintenance mode and thus should not be changed until it is removed!

onebone · 2022-01-19T13:34:48Z

lib/src/main/java/me/onebone/toolbar/CollapsingToolbar.kt

+		return left
+	}
+
+	// TODO: A strange jump in snap speed is often observed


Can you explain this issue?

This is a bug I mentioned. You can see a strange expanding before the snap will applied. This sometimes works even in wrong direction. Look at the demonstration GIF:

When I release the finger you can see that snapping is moving in wrong direction (to top) before it finally snap to full size.

I tried to debug it and I suppose that a bug is in implementation of EnterAlwaysCollapsedNestedScrollConnection#onPostScroll() method. Unfortunately I don't know how NestedScrollConnection and math in EnterAlwaysCollapsedNestedScrollConnection work. I will be glad if you help me with it. Try to build an example app and reproduce this bug.

RareScrap · 2022-03-21T10:19:09Z

Sorry guys, I was too busy to done with it but now I have some time :)

jkwiecien · 2022-05-17T09:01:41Z

Rooting for that snapping guys! :)

GIGAMOLE · 2023-05-08T14:50:22Z

Hi, I created a newer version of the snapping feature here: #83

PR contains the recorded gifs and you can test it by yourself. For me, it seems like everything is working fine.

RareScrap added 2 commits November 7, 2021 15:44

appbar snapping implementation for ExitUntilCollapsed scroll strategy

ab42e1b

+ Extracted defaults

appbar snap implementation

6a8db15

RareScrap force-pushed the feature/snap branch from ca373fe to 6a8db15 Compare November 7, 2021 20:11

onebone requested changes Nov 14, 2021

View reviewed changes

lib/src/main/java/me/onebone/toolbar/CollapsingToolbar.kt Outdated Show resolved Hide resolved

onebone added the feature New feature or request label Nov 16, 2021

moved snap processing from CollapsingToolbarState to CollapsingToolba…

3bad446

…rScaffoldState + Defaults naming refactoring + TODOs added

RareScrap force-pushed the feature/snap branch from 34b58cc to 3bad446 Compare December 1, 2021 06:03

onebone requested changes Dec 5, 2021

View reviewed changes

onebone requested changes Dec 8, 2021

View reviewed changes

lib/src/main/java/me/onebone/toolbar/CollapsingToolbarScaffold.kt Outdated Show resolved Hide resolved

ChristopherKlammt reviewed Dec 21, 2021

View reviewed changes

lib/src/main/java/me/onebone/toolbar/CollapsingToolbarScaffold.kt Outdated Show resolved Hide resolved

RareScrap and others added 9 commits December 23, 2021 22:03

added end line

68f16b8

moved toolbar expand/collapse methods to CollapsingToolbarState

14acae5

removed redundant offsetY passing

ef7cbdf

processSnap and processOffsetSnap moved into extensions

f2842b7

refactored argument of collapseOffset and expandOffset

5ba771d

refactored ugly math

241ccab

renamed SnapStrategy to SnapConfig

a9472a6

Add check whether snap is needed

3d27659

- when toolbar is expanded or collapsed fully, do not perform snap (as it is not needed), this is the case, when progress == 0 or 1 - this improves performance and fixes a bug with horizontal scrolling in the CollapsingToolbarScaffold content

Merge pull request #1 from ChristopherKlammt/feature/snap

2a534d6

Add check whether snap to perform snap

samirabdo123 approved these changes Jan 11, 2022

View reviewed changes

Marx6 approved these changes Jan 19, 2022

View reviewed changes

Marx6 reviewed Jan 19, 2022

View reviewed changes

onebone requested changes Jan 19, 2022

View reviewed changes

onebone linked an issue Jan 23, 2022 that may be closed by this pull request

Snap on release #14

Open

100cracker69 approved these changes Jun 26, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snapping #16

Snapping #16

RareScrap commented Nov 7, 2021

onebone commented Nov 14, 2021 •

edited

Loading

RareScrap commented Nov 25, 2021

onebone commented Nov 25, 2021

RareScrap commented Dec 1, 2021

onebone commented Dec 2, 2021

onebone Dec 5, 2021

RareScrap Dec 23, 2021

Marx6 Jan 19, 2022

RareScrap Jan 20, 2022

ChristopherKlammt commented Jan 17, 2022

RareScrap commented Jan 17, 2022

Marx6 left a comment

Marx6 Jan 19, 2022

onebone Jan 19, 2022

onebone Jan 19, 2022

onebone Jan 19, 2022

RareScrap Jan 20, 2022

RareScrap commented Mar 21, 2022

jkwiecien commented May 17, 2022

GIGAMOLE commented May 8, 2023

Snapping #16

Are you sure you want to change the base?

Snapping #16

Conversation

RareScrap commented Nov 7, 2021

onebone commented Nov 14, 2021 • edited Loading

RareScrap commented Nov 25, 2021

onebone commented Nov 25, 2021

RareScrap commented Dec 1, 2021

onebone commented Dec 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopherKlammt commented Jan 17, 2022

RareScrap commented Jan 17, 2022

Marx6 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RareScrap commented Mar 21, 2022

jkwiecien commented May 17, 2022

GIGAMOLE commented May 8, 2023

onebone commented Nov 14, 2021 •

edited

Loading