chore: Update MediaGallery to Uno.Sdk 5.5

unoplatform / Uno.Samples

A collection of code samples for the Uno Platform

http://platform.uno/

Other

221 stars 111 forks source link

chore: Update MediaGallery to Uno.Sdk 5.5 #843

Open eriklimakc opened 1 month ago

eriklimakc commented 1 month ago

Issue: https://github.com/unoplatform/Uno.Samples/issues/828

Update Sdk to 5.4.5 Some files are using tab and others space for indentation. Changes for all to use space.

eriklimakc commented 1 month ago

@agneszitte @kazo0 Should we remove MacOS and Windows from the Platforms folder, launchSettings.json (only Windows), and from <TargetFrameworks>, since this app is Android and iOS only?

https://github.com/unoplatform/Uno.Samples/blob/19d039de6cdfa48d343ac0913f591d99481c5b50/UI/MediaGallery/src/MediaGallerySample/MediaGallery/MediaFileType.cs#L1

agneszitte commented 1 month ago

@agneszitte @kazo0 Should we remove MacOS and Windows from the Platforms folder, launchSettings.json (only Windows), and from <TargetFrameworks>, since this app is Android and iOS only?

https://github.com/unoplatform/Uno.Samples/blob/19d039de6cdfa48d343ac0913f591d99481c5b50/UI/MediaGallery/src/MediaGallerySample/MediaGallery/MediaFileType.cs#L1

@eriklimakc I think we can keep only the platforms that are needed here. What do you think @kazo0 ? Also as the class is supposed to work on Catalyst as well, shouldn't it be possible to keep MacOS ? I have not checked all the details yet, but I would like your first thoughts on it @eriklimakc please

eriklimakc commented 1 month ago

@eriklimakc I think we can keep only the platforms that are needed here. What do you think @kazo0 ? Also as the class is supposed to work on Catalyst as well, shouldn't it be possible to keep MacOS ? I have not checked all the details yet, but I would like your first thoughts on it @eriklimakc please

@agneszitte Oh yes, I didn't realise that __IOS__ would also include mac. So yes, I think only Windows should be removed then. Right @kazo0?

agneszitte commented 1 month ago

@eriklimakc I think we can keep only the platforms that are needed here. What do you think @kazo0 ? Also as the class is supposed to work on Catalyst as well, shouldn't it be possible to keep MacOS ? I have not checked all the details yet, but I would like your first thoughts on it @eriklimakc please

@agneszitte Oh yes, I didn't realise that __IOS__ would also include mac. So yes, I think only Windows should be removed then. Right @kazo0?

@eriklimakc no __IOS__ on this one normally does not include macos/catalyst cf. https://platform.uno/docs/articles/platform-specific-csharp.html#if-conditionals

eriklimakc commented 1 month ago

@eriklimakc no __IOS__ on this one normally does not include macos/catalyst cf. https://platform.uno/docs/articles/platform-specific-csharp.html#if-conditionals

@agneszitte That is funny, because when I change that dropdown to M⁠acCatalyst it continues showing the code as enabled:

While if I choose Windows the code shows as not enabled/ignored:

cc @kazo0

agneszitte commented 3 weeks ago

This PR will need to be rebased with latest and updated to latest 5.5 stable version Related new issue: https://github.com/unoplatform/Uno.Samples/issues/848

eriklimakc commented 1 week ago

Can someone test iOS, please?

@agneszitte @kazo0