Change default theme from a const object to a function that returns the object

[lyzhan7]

Platforms Impacted

• iOS
• macOS
• win32 (Office)
• windows
• android

Description of changes

Update the default theme fluent object to be a method that returns an object.

This issue is blocking #2481:

• the updated designs for iOS tokens include some new tokens that aren't in the default theme
• mapPipelineToTheme.ios.ts was updated to reflect these designs
• however, the default theme is still getting created and is trying to running the iOS code for mapPipelineToTheme, which causes failures because the iOS alias token set now includes tokens that aren't in the default theme (win32) token set
• changing the default theme from a const object to a method prevents the default theme from getting created automatically

Verification

No visual changes
Check if CI passes

Pull request checklist

This PR has considered (when applicable):

• Automated Tests
• Documentation and examples
• Keyboard Accessibility
• Voiceover
• Internationalization and Right-to-left Layouts

[lyzhan7]

Maybe to be done in a future PR - we could potentially combine all three defaultFluentTheme methods into one method that takes the appearance as an argument. This is what we do on iOS and I think android

[ghost]

Hello @lyzhan7!

Because this pull request has the AutoMerge :repeat: label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 4 hours 21 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[ankraj12]

Just to understand, the issue you hit is the same as android integrations which failed due to android being a different alias color set. The changes proposed in #2190 should fix this for iOS. Are you planning to pick that as part of ongoing color work? In which case this PR could be redundant.

[ankraj12]

For the question over change

[lyzhan7]

Just to understand, the issue you hit is the same as android integrations which failed due to android being a different alias color set. The changes proposed in #2190 should fix this for iOS. Are you planning to pick that as part of ongoing color work? In which case this PR could be redundant.

I see, I didn't know Android hit the same issue. Yes I'm about to update my iOS color tokens PR to match what Android did - I can see how that would also get rid of the issue, but regardless I think this is still a good change to make, because right now we're automatically creating the default theme on iOS. I think updating the iOS color tokens PR to move getTokens into one shared place won't prevent the default theme from getting created at startup, it'll just make the default theme get its values from iOS (my understanding is that the default theme should get its value from windows tokens)

[lyzhan7]

Just updated the iOS color tokens PR to move/rename apple-theme/getiOSTokens.ts -> theme-tokens/getTokens.ios.ts and ran into a separate but related issue - right now we're creating all three variants of the default theme (light mode, dark mode, and high contrast mode) when iOS boots, so after the rename/move we hit an error in getTokens.ios.ts that high contrast is not available on iOS. Android didn't run into this because it doesn't have this error handling. I don't think we should remove the error handling on the iOS side to get this unblocked, updating how the default theme is created still seems like the right change to make overall so right now this PR is still blocking the iOS color tokens PR.

[ankraj12]

(my understanding is that the default theme should get its value from windows tokens)

Just to understand, the issue you hit is the same as android integrations which failed due to android being a different alias color set. The changes proposed in #2190 should fix this for iOS. Are you planning to pick that as part of ongoing color work? In which case this PR could be redundant.

I see, I didn't know Android hit the same issue. Yes I'm about to update my iOS color tokens PR to match what Android did - I can see how that would also get rid of the issue, but regardless I think this is still a good change to make, because right now we're automatically creating the default theme on iOS. I think updating the iOS color tokens PR to move getTokens into one shared place won't prevent the default theme from getting created at startup, it'll just make the default theme get its values from iOS (my understanding is that the default theme should get its value from windows tokens)

I didn't get a chance to re-review the code again, but if this changes what default-theme gets created to always create from windows, I believe this would be incorrect. My understanding is that default-theme should

Just to understand, the issue you hit is the same as android integrations which failed due to android being a different alias color set. The changes proposed in #2190 should fix this for iOS. Are you planning to pick that as part of ongoing color work? In which case this PR could be redundant.

I see, I didn't know Android hit the same issue. Yes I'm about to update my iOS color tokens PR to match what Android did - I can see how that would also get rid of the issue, but regardless I think this is still a good change to make, because right now we're automatically creating the default theme on iOS. I think updating the iOS color tokens PR to move getTokens into one shared place won't prevent the default theme from getting created at startup, it'll just make the default theme get its values from iOS (my understanding is that the default theme should get its value from windows tokens)

Sorry I didn't get chance to review this part fully again post your response, but if this PR change what's the default theme returned for an endpoint to "always" by widows tokens - I believe this is incorrect. My understanding is that default-theme should return the platform default theme set of color values. Could you re-check and confirm if this behaviour still works?

[ankraj12]

Just updated the iOS color tokens PR to move/rename apple-theme/getiOSTokens.ts -> theme-tokens/getTokens.ios.ts and ran into a separate but related issue - right now we're creating all three variants of the default theme (light mode, dark mode, and high contrast mode) when iOS boots, so after the rename/move we hit an error in getTokens.ios.ts that high contrast is not available on iOS. Android didn't run into this because it doesn't have this error handling. I don't think we should remove the error handling on the iOS side to get this unblocked, updating how the default theme is created still seems like the right change to make overall so right now this PR is still blocking the iOS color tokens PR.

got that! my only concern would remain if what's the default theme returning for android post this change per other comment.

[lyzhan7]

Asked about whether the default-theme should always be using windows tokens or be using tokens depending on the platform offline and I misunderstood - it should be using the right tokens for the platform, not always windows tokens. I think this raises some other questions: currently the default theme creation code assumes that there is high contrast mode, and tries to get high contrast tokens from the pipeline - but on iOS/Android we don't have a high contrast mode. I've started a thread for this offline.

On Android we're just returning the light mode theme when we're trying to create the high contrast default theme, but on iOS we throw an error, which is why I still need this PR to unblock the iOS color token work. I could also make iOS return its light mode tokens when we try to access high contrast theme instead of throwing an error, that's another way to unblock the iOS color tokens PR, but I still think the right thing is to not be creating the default theme automatically on boot.

This PR does not make any changes to which platform tokens get returned for the default theme. The iOS color alias does change this though - currently on iOS the default theme is trying to access windows tokens. After my change, the default theme on iOS will be trying to access iOS tokens.

I don't have any planned iterations to make to this PR - please let me know if there was a change you were expecting

[rurikoaraki]

Asked about whether the default-theme should always be using windows tokens or be using tokens depending on the platform offline and I misunderstood - it should be using the right tokens for the platform, not always windows tokens. I think this raises some other questions: currently the default theme creation code assumes that there is high contrast mode, and tries to get high contrast tokens from the pipeline - but on iOS/Android we don't have a high contrast mode. I've started a thread for this offline.

On Android we're just returning the light mode theme when we're trying to create the high contrast default theme, but on iOS we throw an error, which is why I still need this PR to unblock the iOS color token work. I could also make iOS return its light mode tokens when we try to access high contrast theme instead of throwing an error, that's another way to unblock the iOS color tokens PR, but I still think the right thing is to not be creating the default theme automatically on boot.

This PR does not make any changes to which platform tokens get returned for the default theme. The iOS color alias does change this though - currently on iOS the default theme is trying to access windows tokens. After my change, the default theme on iOS will be trying to access iOS tokens.

I don't have any planned iterations to make to this PR - please let me know if there was a change you were expecting

I might have phrased this in an unclear way but, for clarification, currently the default theme is meant to always use web tokens.

I think that it could be changed to be platform specific in the future, but it does not work that way today and is not meant to work that way right now.

[ankraj12]

Asked about whether the default-theme should always be using windows tokens or be using tokens depending on the platform offline and I misunderstood - it should be using the right tokens for the platform, not always windows tokens. I think this raises some other questions: currently the default theme creation code assumes that there is high contrast mode, and tries to get high contrast tokens from the pipeline - but on iOS/Android we don't have a high contrast mode. I've started a thread for this offline.

On Android we're just returning the light mode theme when we're trying to create the high contrast default theme, but on iOS we throw an error, which is why I still need this PR to unblock the iOS color token work. I could also make iOS return its light mode tokens when we try to access high contrast theme instead of throwing an error, that's another way to unblock the iOS color tokens PR, but I still think the right thing is to not be creating the default theme automatically on boot.

This PR does not make any changes to which platform tokens get returned for the default theme. The iOS color alias does change this though - currently on iOS the default theme is trying to access windows tokens. After my change, the default theme on iOS will be trying to access iOS tokens.

I don't have any planned iterations to make to this PR - please let me know if there was a change you were expecting

Thanks for checking on this. For Android on high contrast, I did not try fixing/debugging on the flow as it's not applicable. If you have a use case for iOS on it that you would like to handle differently, that sounds good.