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

Commonize :compose:material3:adaptive modules #1468

Merged
merged 12 commits into from
Aug 13, 2024

Conversation

terrakok
Copy link
Member

@terrakok terrakok commented Jul 29, 2024

Commonize and configure publishing for

  • :compose:material3:adaptive:adaptive
  • :compose:material3:adaptive:adaptive-layout
  • :compose:material3:adaptive:adaptive-navigation
  • :window:window-core
    modules.

Added new method to non-android source set to get WindowAdaptiveInfo.

Fixes https://youtrack.jetbrains.com/issue/CMP-5817

Testing

Checked it in the MPP demo app:
image

Release Notes

Features - Multiple Platforms

  • Commonized :compose:material3:adaptive:adaptive, :compose:material3:adaptive:adaptive-layout, :compose:material3:adaptive:adaptive-navigation and :window:window-core modules

@terrakok terrakok requested a review from igordmn July 29, 2024 12:13
@igordmn
Copy link
Collaborator

igordmn commented Jul 29, 2024

Just copy-pasted material3/adaptive directory from the androidx to JetBrains fork

Please specify the source commit. is it androidx/compose-material3/1.3.0-beta03?

@terrakok terrakok force-pushed the k.tskh/material3-adaptive-upstream branch 4 times, most recently from 9b2b35d to 5b1a50a Compare July 30, 2024 12:08
@terrakok terrakok changed the title Apply material3/adaptive from androidx Commonize :compose:material3:adaptive:adaptive module Jul 30, 2024
@terrakok terrakok force-pushed the k.tskh/material3-adaptive-upstream branch from 06914eb to 1a9612c Compare July 31, 2024 12:54
@terrakok terrakok changed the title Commonize :compose:material3:adaptive:adaptive module Commonize :compose:material3:adaptive modules Jul 31, 2024
@terrakok terrakok force-pushed the k.tskh/material3-adaptive-upstream branch from d707981 to 76106df Compare July 31, 2024 13:50
@terrakok terrakok requested a review from MatkovIvan July 31, 2024 16:37
MULTIPLATFORM.md Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised to see no dependencies to a navigation library here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is quite small module with independent logic how to behave when a screen has different layouts

@@ -292,6 +292,7 @@ WEAR_TILES = { group = "androidx.wear.tiles", atomicGroupVersion = "versions.WEA
WEAR_WATCHFACE = { group = "androidx.wear.watchface", atomicGroupVersion = "versions.WEAR_WATCHFACE" }
WEBKIT = { group = "androidx.webkit", atomicGroupVersion = "versions.WEBKIT" }
WINDOW = { group = "androidx.window", atomicGroupVersion = "versions.WINDOW" }
WINDOW_CORE = { group = "org.jetbrains.window.core", atomicGroupVersion = "versions.WINDOW", overrideInclude = [ ":window:window-core" ] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"versions.WINDOW" is already 1.6.0-alpha01 in AOSP. Shouldn't we update it here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't touch the version. As I understand the version was set in the commit we based our fork on

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because we don't merge window and libversions to jb-main, they can be old. The actual state is in integration, but as with any other lib we merge only released commits to jb-main

# We use artifactRedirecting not only for Compose libs:
artifactRedirecting.androidx.collection.version=1.4.0
artifactRedirecting.androidx.annotation.version=1.8.0
artifactRedirecting.androidx.lifecycle.version=2.8.0
artifactRedirecting.androidx.navigation.version=2.8.0-beta05
artifactRedirecting.androidx.savedstate.version=1.2.1
# Look for `WINDOW` in libraryversions.toml
artifactRedirecting.androidx.window.core.version=1.3.0-alpha01
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stable 1.3.0 was release a few months ago. Any reason to use alpha?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't touch the version. As I understand the version was set in the commit we based our fork on

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window subfolder wasn't involved in specific version selection, If it's outdated, please merge the stable state (might be done separately)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm almost sure that this version doesn't match to the commit/state of the code. Blocking the merge before explicit check

MULTIPLATFORM.md Show resolved Hide resolved
Copy link
Member

@MatkovIvan MatkovIvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, no blockers. Please recheck the versions on our CI and redirecting artifacts before merge

# We use artifactRedirecting not only for Compose libs:
artifactRedirecting.androidx.collection.version=1.4.0
artifactRedirecting.androidx.annotation.version=1.8.0
artifactRedirecting.androidx.lifecycle.version=2.8.0
artifactRedirecting.androidx.navigation.version=2.8.0-beta05
artifactRedirecting.androidx.savedstate.version=1.2.1
# Look for `WINDOW` in libraryversions.toml
artifactRedirecting.androidx.window.core.version=1.3.0-alpha01
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm almost sure that this version doesn't match to the commit/state of the code. Blocking the merge before explicit check

visibilityThreshold = 0.1f,
delayedRatio = delayedRatio,
)
dampingRatio = 0.7f,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's reset formatting to avoid merge conflicts (in the other files too)

gradle.properties Outdated Show resolved Hide resolved
@@ -292,6 +292,7 @@ WEAR_TILES = { group = "androidx.wear.tiles", atomicGroupVersion = "versions.WEA
WEAR_WATCHFACE = { group = "androidx.wear.watchface", atomicGroupVersion = "versions.WEAR_WATCHFACE" }
WEBKIT = { group = "androidx.webkit", atomicGroupVersion = "versions.WEBKIT" }
WINDOW = { group = "androidx.window", atomicGroupVersion = "versions.WINDOW" }
WINDOW_CORE = { group = "org.jetbrains.window.core", atomicGroupVersion = "versions.WINDOW", overrideInclude = [ ":window:window-core" ] }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because we don't merge window and libversions to jb-main, they can be old. The actual state is in integration, but as with any other lib we merge only released commits to jb-main

MULTIPLATFORM.md Show resolved Hide resolved
It was pinned in the original `androidx.compose.material3.adaptive` module dependencies
@terrakok terrakok force-pushed the k.tskh/material3-adaptive-upstream branch from f10cc9f to a986c98 Compare August 13, 2024 10:48
gradle.properties Outdated Show resolved Hide resolved
gradle.properties Outdated Show resolved Hide resolved
@terrakok terrakok requested a review from igordmn August 13, 2024 12:47
@terrakok terrakok merged commit 16b3858 into jb-main Aug 13, 2024
6 checks passed
@terrakok terrakok deleted the k.tskh/material3-adaptive-upstream branch August 13, 2024 13:28
mazunin-v-jb pushed a commit that referenced this pull request Aug 14, 2024
Commonize and configure publishing for 
 - `:compose:material3:adaptive:adaptive`
 - `:compose:material3:adaptive:adaptive-layout`
 - `:compose:material3:adaptive:adaptive-navigation`
 - `:window:window-core`
modules.

Added new method to non-android source set to get `WindowAdaptiveInfo`. 

Fixes https://youtrack.jetbrains.com/issue/CMP-5817

## Testing
Checked it in the MPP demo app:
<img width="600" alt="image"
src="https://github.com/user-attachments/assets/e2dbe01f-1481-4e4f-ab9d-e2641bc441d0">



## Release Notes
### Features - Multiple Platforms
- Commonized `:compose:material3:adaptive:adaptive`,
`:compose:material3:adaptive:adaptive-layout`,
`:compose:material3:adaptive:adaptive-navigation` and
`:window:window-core` modules
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

Successfully merging this pull request may close these issues.

3 participants