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

Material icons extended only being published for desktop #2106

Closed
luca992 opened this issue Jun 9, 2022 · 23 comments · Fixed by #2133
Closed

Material icons extended only being published for desktop #2106

luca992 opened this issue Jun 9, 2022 · 23 comments · Fixed by #2133

Comments

@luca992
Copy link
Contributor

luca992 commented Jun 9, 2022

I was having trouble using the extended material icons in my common code. It turns out it is only being built/published for desktop.

You can verify here:
https://maven.pkg.jetbrains.space/public/p/compose/dev/org/jetbrains/compose/material/

Material icons core does not have this issue.

@igordmn
Copy link
Collaborator

igordmn commented Jun 17, 2022

We only publish it for desktop. For android it should be redirected to the Jetpack Compose published version.

I just checked Compose 1.1.1, and it works fine, if we add icons this way (in MPP-style project):

kotlin {
    ...
    sourceSets {
        named("commonMain") {
            dependencies {
                ...
                api(compose.materialIconsExtended)
            }
        }
    }
}

and use Icons.Filled.BlurOn

@igordmn
Copy link
Collaborator

igordmn commented Jun 17, 2022

@luca992
Copy link
Contributor Author

luca992 commented Jun 17, 2022

Okay, if that's on purpose, why? The basic icon set is published for all targets as I mentioned. Shouldn't the extended icons be published for all targets as well?

@luca992
Copy link
Contributor Author

luca992 commented Jun 17, 2022

We only publish it for desktop. For android it should be redirected to the Jetpack Compose published version.

I just checked Compose 1.1.1, and it works fine, if we add icons this way (in MPP-style project):

kotlin {
    ...
    sourceSets {
        named("commonMain") {
            dependencies {
                ...
                api(compose.materialIconsExtended)
            }
        }
    }
}

and use Icons.Filled.BlurOn

@igordmn

Sorry, what I reported was somewhat incorrect.

The project will build with implementation(compose.materialIconsExtended) in commonMain. But, Intellij will fail to resolve any extended icon in the commonMain source set. Which is just really annoying plus code completion for the extended icons doesn't work.

to work around that issue I added implementation("org.jetbrains.compose.material:material-icons-extended-desktop:_") to both jvm and android dependencies and everything works as it should.

However, I have been trying out the ios support on the alpha build and it would be nice to have the icons published for all the targets

@Antimonit
Copy link

I just ran into the same issue as @luca992.

Icons.Default.Add is in the material-icons-core package.
Icons.Default.Remove is in the material-icons-extended package.

  • With api(compose.materialIconsExtended)
    • Icons.Default.Remove is highlighted red and autocomplete for icons from the extended package does not work.
  • With api(compose("org.jetbrains.compose.material:material-icons-extended-desktop"))
    • Icons.Default.Remove is correctly recognized.

In both cases, the app compiles just fine but IntelliJ IDEA highlighting and autocomplete are broken in the first scenario.

igordmn added a commit that referenced this issue Jun 20, 2022
Fixes #2106

Also will add a task on CI
@igordmn
Copy link
Collaborator

igordmn commented Jun 20, 2022

I have been trying out the ios support on the alpha build and it would be nice to have the icons published for all the targets

Oh, I understand. Publishing for iOS and JS is indeed not enabled. It will be fixed. Publishing for Android will remain the same (artifacts will point to the artifacts published by Google)

@Thomas-Vos
Copy link
Contributor

@igordmn I think when these packages (icons extended, material3, etc) are published for all targets that the following issue may be fixed as well. It is about autocomplete not working in common source sets. #2032 (see comments). It seems to only affect the packages that are not published for all targets.

igordmn added a commit that referenced this issue Jun 29, 2022
@igordmn
Copy link
Collaborator

igordmn commented Jun 29, 2022

It should be fixed in 1.2.0-alpha01-dev731. Could you try it?

@luca992
Copy link
Contributor Author

luca992 commented Jun 29, 2022

1.2.0-alpha01-dev731 is working for me in common and building for jvm and android. Thanks!

@lucasmeneghin
Copy link

material-icons-extended-js is still missing from https://maven.pkg.jetbrains.space/public/p/compose/dev/org/jetbrains/compose/material/

@igordmn
Copy link
Collaborator

igordmn commented Jul 5, 2022

material-icons-extended-js is still missing from

Indeed. Thanks for noticing.

Reopening this issue until we fix it for js.

@igordmn igordmn reopened this Jul 5, 2022
@luca992
Copy link
Contributor Author

luca992 commented Sep 13, 2022

is there any eta on js? @igordmn

@lucasmeneghin
Copy link

can we have this fixed? :(
@igordmn

@eygraber
Copy link
Contributor

eygraber commented Nov 30, 2022

@igordmn is there anything to do here besides making sure that artifact gets published correctly?

Edit: I just published to maven local and it seems to work fine

@igordmn
Copy link
Collaborator

igordmn commented Nov 30, 2022

@igordmn is there anything to do here besides making sure that artifact gets published correctly?

Probably not, but I can't tell for sure. This task should publish icons for all platforms (into the local directory). This issue isn't in our near plans, but if someone will make a PR with the fix, ping me in the PR, and mention this issue, I will review and merge.

@eygraber
Copy link
Contributor

I'll see if I can find anything.

Would it be possible to manually build and deploy the artifacts for existing versions (1.2.1, 1.3.0-beta, etc...)?

@igordmn
Copy link
Collaborator

igordmn commented Nov 30, 2022

possible to manually build

No, it requires much more efforts comparing to just publishing for the next versions (our CI isn't configured for this)

@eygraber
Copy link
Contributor

How does CI run publishComposeJbExtendedIcons?

Without seeing that, my best guess would be that compose.platforms either isn't set or doesn't specify js when running publishExtendedIcons

@igordmn
Copy link
Collaborator

igordmn commented Nov 30, 2022

Here is the CI task (you can create an account in this TeamCity if you don't have one)

image
image
image

@igordmn
Copy link
Collaborator

igordmn commented Nov 30, 2022

I looked on the other task, and found out that for JS we call publish tasks separately with an additional argument. Added a new task:

image

And ran the new 1.3.0-beta

@eygraber
Copy link
Contributor

Awesome thank you!

@igordmn
Copy link
Collaborator

igordmn commented Nov 30, 2022

Here there are 🎉.

@igordmn igordmn closed this as completed Nov 30, 2022
@okushnikov
Copy link
Collaborator

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

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

Successfully merging a pull request may close this issue.

7 participants