Skip to content

Code Quality: Upgrade from legacy WCT to WCT 8.x #16705

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

Closed
Lamparter opened this issue Jan 12, 2025 · 8 comments · Fixed by #16741
Closed

Code Quality: Upgrade from legacy WCT to WCT 8.x #16705

Lamparter opened this issue Jan 12, 2025 · 8 comments · Fixed by #16741

Comments

@Lamparter
Copy link
Contributor

Description

Currently, Files has a large dependency on the old CommunityToolkit.WinUI.UI.* packages.
These do not support NativeAOT and are deprecated.
They should be upgraded to WCT 8.x

Concerned code

  • Lots of classes rely on legacy helpers and converters

Gains

  • Native AOT compilation / trimming
  • Better experience as newer packages are optimised and up to date

Requirements

  • Upgrade existing infrastructure
  • Port to new or recreated controls
  • Help along in collaboration with WCT to port what Files needs to 8.x

Comments

@0x5bfa wrote an amazing list of everything that need porting: 0x5bfa/Files.wiki:Migrate-WCT-from-7.x-to-8.x

@yaira2 yaira2 moved this from 🆕 New to 🔖 Ready to build in Files task board Jan 12, 2025
@Lamparter

This comment has been minimized.

@Jay-o-Way
Copy link
Contributor

If needed, @niels9001 knows all about this subject

@Lamparter
Copy link
Contributor Author

Thanks! I'm looking forward to working with the Community Toolkit team on what to port/rewrite for use in 8.x

However, right now I'm facing this issue.

Unfortunately, there's an error that occurs with the DP extensions (an ambiguous reference between the extension FindAscending in both 7.x and 8.x)
And every WCT 8.x package relies on the new CommunityToolkit.WinUI.Extensions package that is causing the issue.
So every control must be ported at the same time
Except since BladeView hasn't been ported yet / a different implementation used, upgrading is impossible as of now

@Jay-o-Way
Copy link
Contributor

Fun fact about the BoolToVisibilityConverter: its function is embedded in WinUi 3 😊

@0x5bfa
Copy link
Member

0x5bfa commented Jan 20, 2025

Fun fact about the BoolToVisibilityConverter: its function is embedded in WinUi 3 😊

Yeah I was surprised when I was allowed to set the boolean to a Visibility property and it worked perfectly. I wonder when it was added.

@0x5bfa
Copy link
Member

0x5bfa commented Jan 24, 2025

Can we work on this from Files.App.Controls?

@Lamparter
Copy link
Contributor Author

Lamparter commented Jan 24, 2025

No
It's literally entirely impossible, because of the blocker

@yaira2 yaira2 moved this from 🔖 Ready to build to 🏗 In progress in Files task board Jan 27, 2025
@0x5bfa
Copy link
Member

0x5bfa commented Feb 6, 2025

FYI https://learn.microsoft.com/nl-nl/windows/uwp/xaml-platform/x-bind-markup-extension#remarks

Starting in Windows 10, version 1607, the XAML framework provides a built in Boolean to Visibility converter. The converter maps true to the Visible enumeration value and false to Collapsed so you can bind a Visibility property to a Boolean without creating a converter.

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

Successfully merging a pull request may close this issue.

3 participants