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

Removal of the AppAction icon extension on Android #14235

Conversation

moljac
Copy link
Contributor

@moljac moljac commented Mar 28, 2023

Description of Change

Context: Android will not render AppAction icons with provided filename (no error, no runtime crash), while other platforms support extensionless icon names.

Issues Fixed

Fixes #9234

Details: #9234 (comment)

@moljac moljac requested a review from mattleibow March 28, 2023 13:09
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Platform-specific changes really should be in the android file. Rather remove the extension when creating the native action as this behaviour is a bit unexpected here.

@moljac
Copy link
Contributor Author

moljac commented Mar 29, 2023

Platform-specific changes really should be in the android file. Rather remove the extension when creating the native action as this behaviour is a bit unexpected here.

these blocks were a bit confusing and misleading (with #preprocessor), so I thought it is OK to place code in *.shared.cs

https://github.com/dotnet/maui/blob/main/src/Essentials/src/AppActions/AppActions.shared.cs#L42-L69

https://github.com/dotnet/maui/blob/main/src/Essentials/src/AppActions/AppActions.shared.cs#L42-L69

@moljac
Copy link
Contributor Author

moljac commented Mar 29, 2023

OK. Code moved to AppActions.android.cs.

I hope it is better

@Redth
Copy link
Member

Redth commented Mar 31, 2023

@moljac you are missing a using System.IO; in the file.

Once that's fixed this should be good to go.

@jfversluis
Copy link
Member

/azp run

@azure-pipelines

This comment was marked as off-topic.

@jfversluis
Copy link
Member

Whoops looks like the build was actually fine but the CLA was still giving issues. Did we get anywhere with that @rmarinho ?

Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

I see there is a change to the maui.aar without any java code changes. Can we revert that?

@rmarinho rmarinho closed this Apr 3, 2023
@rmarinho rmarinho reopened this Apr 3, 2023
@rmarinho
Copy link
Member

rmarinho commented Apr 3, 2023

I still didn't got a response @jfversluis , seems close and reopen would work but it isn't

@moljac
Copy link
Contributor Author

moljac commented Apr 4, 2023

I still didn't got a response @jfversluis , seems close and reopen would work but it isn't

Resopnse from me?
Do I need to do something?

@jfversluis
Copy link
Member

jfversluis commented Apr 4, 2023

@moljac not from you! From people in charge of the CLA stuff :D but it seems closing and reopening did something because the CLA check is happy now.

Just the comment from Matt above needs addressing now!

@moljac
Copy link
Contributor Author

moljac commented Apr 4, 2023

Just the comment from Matt above needs addressing now!

why is this maui.aar commited? It is being changed on every local build.

I know - tooling. I bit confusing. Could it be added to .gitginore?

@mattleibow mattleibow enabled auto-merge (squash) April 4, 2023 20:25
@mattleibow mattleibow self-requested a review April 4, 2023 20:25
@mattleibow mattleibow merged commit b324e2c into dotnet:main Apr 4, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AddAppAction icons don't work with MauiImages in the Resources/Images folder on Android
6 participants