Skip to content
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

Contribute this to AndroidX? #2

Open
jamesward opened this issue Nov 15, 2022 · 5 comments
Open

Contribute this to AndroidX? #2

jamesward opened this issue Nov 15, 2022 · 5 comments

Comments

@jamesward
Copy link

This is great! Not sure yet if a PR back to AndroidX would be accepted but if it were to be, would you be interested in unforking this?

@veyndan
Copy link
Collaborator

veyndan commented Nov 16, 2022

Thanks! Yes, I'd be very interested in upstreaming these changes up to AndroidX (partially or fully). For a quick breakdown of what that would involve:

  • To upstream the paging-common changes, our changes on the androidx-main branch should be cherry-pickable back to upstream.
  • The additional iOS bindings provided in paging-runtime should just be directly copy-able from its module.

@veyndan
Copy link
Collaborator

veyndan commented Nov 16, 2022

If there is a possibility to upstream these changes back to AndroidX, I'd obviously be happy to help with the process of contributing it upstream. If there are any technical concerns about having it upstreamed, I'd also be happy to talk through them!

@jamesward
Copy link
Author

Thanks for the details and openness to contributing this back. I'll post here when/if this is something the team can take on.

@veyndan veyndan pinned this issue Jan 6, 2023
veyndan pushed a commit that referenced this issue Jan 6, 2023
Now with toml fixes!

Test: ./gradlew bOS --dry-run
Change-Id: I888372d181532ff55cf51475bf04ab039be2c773
veyndan pushed a commit that referenced this issue Jan 6, 2023
This reverts commit d25c04a.

Reason for revert: b/216514062; breaks local runs on osx

Change-Id: Iab86601df3f7603c10a90e235d0b3239ca5ae452
veyndan pushed a commit that referenced this issue Jan 6, 2023
veyndan pushed a commit that referenced this issue Jan 6, 2023
This CL fixes a bug where calling `XType.getTypeElement().className`
results in a `ClassCastException` when called on an array type in
KSP. The underlying issue is that `XType.getTypeElement()` gets
tricked into thinking there's an actual `TypeElement` for an array
type since it has a declaration, `Array` which it thinks is the
appropriate `TypeElement`, which it's not.

This leads to two issues.

  1. Since an array type isn't actually represented as a `ClassName`,
     we get a `ClassCastException` when trying to cast it.
  2. It leads to inconsistencies when compiling in Javac versus KSP.

For the same consistency reason as #2, I've also changed the
implementation for primitive types to return null type element in
KSP since this matches the Javac behavior where there is no valid
ClassName for a primitive type.

Test: XTypeTest
Change-Id: I7e5d8856115d99e666e487c52cc5d9f538149cae
@veyndan
Copy link
Collaborator

veyndan commented Mar 24, 2023

For anyone following along, there's been talks with the AndroidX folks.

The immediate plan is to "Ensure paging-common is free of Android and JVM specific API usages" upstream (i.e., within AndroidX itself). With the completion of that ticket, this doesn't mean that the upstream paging-common artifact will be multiplatform. It just means that making upstream multiplatform will be easier in the future.

As a consumer of Multiplatform Paging, nothing much will change. The only benefit that I could see is that new versions of AndroidX Paging will be tracked here a bit faster, as the lower the the delta between upstream and my fork, the less work it is to track a new release.

veyndan pushed a commit that referenced this issue May 5, 2023
This CL adds a hoisted ScrollState to BasicTextField2 to enable horizontal and vertical scroll. Like the existing `BasicTextField`, BTF2 only supports single orientated scrolling. Single line configuration disables `softWrap` and enables horizontal scroll. Multi-line configuration does the exact opposite to enable vertical scroll.

Cursor is kept in visible area only when it's shown (focused, editable).

There is also a fix to clip test for `BasicTextField`. The previous test only looked at the area #4, but in fact it needs to check #2, #3, #4 for any spill from BasicTextField.

// BasicTextField is #1
// Wrapping box is all the area visible below
-----
|1|2|
-----
|3|4|
-----

Relnote: N/A
Test: :compose:foundation:foundation:cAT
Change-Id: Iccac198dec2d2908c4c51ddfaf40dcb05515ce70
@veyndan
Copy link
Collaborator

veyndan commented Oct 4, 2023

After much collaboration with the AndroidX Paging team, AndroidX Paging now ships multiplatform artifacts! I'm very happy to see these changes upstream 😄

Where does this leave Multiplatform Paging? You'll (probably) still need it!

  1. AndroidX Paging supports a smaller set of targets than Multiplatform Paging.
    1. For paging-common, if you need to support any one of JS, MinGW, Linux Arm64, tvOS, or watchOS, then you'll still need to use Multiplatform Paging. If you are just targeting JVM, iOS, Linux X64, or macOS, then you can use the official androidx.paging:paging-common artifact.
    2. For paging-compose, if you need to support any target that isn't the JVM, then you'll still need to use Multiplatform Paging. If you are just targeting the JVM, then you can use the official androidx.paging:paging-compose artifact.
  2. AndroidX Paging doesn't provide any integration components with platforms beyond Android. If you want to use AndroidX Paging on iOS, you'll probably want to use paging-runtime-uikit from Multiplatform Paging, which provides a PagingDataAdapter equivalent to iOS.

Mixing and matching artifacts between AndroidX Paging and Multiplatform Paging is valid (as long as they are both on the same version, e.g., 3.3.0-alpha02 and 3.3.0-alpha02-0.4.0 respectively). Therefore, if you are happy with the targets that AndroidX Paging provides, but wish to use paging-runtime-uikit from Multiplatform Paging, then a valid set of dependencies could look like this:

implementation("androidx.paging:paging-common:3.3.0-alpha02")
implementation("app.cash.paging:paging-runtime-uikit:3.3.0-alpha02-0.4.0")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants