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

Refactor FXIOS-8936 Sample Component Library to incorporate UUID-based themes #19740

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

cyndichin
Copy link
Contributor

📜 Tickets

Jira ticket
Github issue

💡 Description

Address build errors in the Sample Component Library due to updates in our theming being based on window UUID. Since this is a sample project and no plans now to support multiwindows, created an extension to minimize this build warnings.

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • If needed, I updated documentation / comments for complex code and public methods
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

import Foundation
import Common

let defaultSampleComponentUUID = UUID(uuidString: "44BA0B7D-097A-484D-8358-91A6E374451D")!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

copied over from client side, but happy to use UUID() if that's preferred

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think in this case the specific UUID value should matter so this should be fine. 👍 That UUID was intended originally for the UI tests but we can use it here, I'm not aware of any conflict it would cause.

@cyndichin
Copy link
Contributor Author

@mattreaganmozilla here's a quick fix to resolve the build errors for the Sample Component Library, but let me know if there's a different path that should be taken!

@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Apr 10, 2024

Messages
📖 Edited 2 files
📖 Created 1 files

Generated by 🚫 Danger Swift against e00e6bc

@cyndichin cyndichin marked this pull request as ready for review April 10, 2024 17:19
@cyndichin cyndichin requested a review from a team as a code owner April 10, 2024 17:19
Comment on lines 19 to 23
window = UIWindow(windowScene: windowScene)
window?.rootViewController = navigationController
window?.makeKeyAndVisible()

guard let window else { return }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Super-minor suggestion: if we store the UIWindow() result first in a local non-optional it avoids the need for the guard (and the optional ? before calling functions).

let newWindow = UIWindow(windowScene: windowScene)
window = newWindow
newWindow.rootViewController = navigationController
// … etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the suggestion, updated!

@cyndichin cyndichin merged commit 4beabfe into main Apr 10, 2024
13 checks passed
@cyndichin cyndichin deleted the cc/FXIOS-8936_Update-Sample-Component-Library branch April 10, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants