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.

Sign up for GitHub

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

Jump to bottom

Migrate app to SwiftUI #801

Merged
merged 16 commits into from
Sep 14, 2024
Merged

Migrate app to SwiftUI #801

merged 16 commits into from
Sep 14, 2024

Conversation

timbms
Copy link
Contributor

@timbms timbms commented Sep 2, 2024 •
edited
Loading

Start with SettingsView

  • Using LabeledContent instead of HStack
  • Proper handling of settingsSendCrashReports when already set
  • Integrated ClientCertificatesView, RTFTextView

@timbms
Migrate SettingsViewController to SwiftUI
c43b7d7 
- Using LabeledContent instead of HStack
- Proper handling of settingsSendCrashReports when already set
- Integrated ClientCertificatesView, RTFTexView
@timbms timbms requested review from weakfl and digitaldan September 2, 2024 16:02
@digitaldan
Copy link
Contributor

Wow, thats great, can't wait to take a look !

@timbms timbms changed the title Migrate SettingsViewController to SwiftUI Migrate app to SwiftUI Sep 5, 2024
@digitaldan
Copy link
Contributor

Hey @timbms hows this coming? Should we review / test it? There are a number of PR's we have both opened, before i start merging i wanted to coordinate with you so i don't cause extra merging headaches for you.

@timbms
Copy link
Contributor Author

timbms commented Sep 12, 2024

Should be good for testing. Please try it
Could no test certificates and notifications as I am not using them

@digitaldan
Copy link
Contributor

So this looks good! I found an unrelated bug in our notification decoding, but once that was fixed, i can list notifications as well as client certs (i'll open a PR for that later today)

Looks like there is some commented out code and the old view classes are still there in "obsolete" named folders, do you want to clean those up before we do a final review, test and merge ?

@timbms
Copy link
Contributor Author

timbms commented Sep 13, 2024

@digitaldan Please have a look

digitaldan
digitaldan reviewed Sep 13, 2024
View reviewed changes
Copy link
Contributor

@digitaldan digitaldan left a comment

Choose a reason for hiding this comment

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

Just some commented out code, but otherwise LGTM,

openHAB/OpenHABSitemapViewController.swift Outdated Show resolved Hide resolved
openHAB/OpenHABSitemapViewController.swift Outdated Show resolved Hide resolved
@timbms
Copy link
Contributor Author

timbms commented Sep 14, 2024

@digitaldan Please have a look

@digitaldan digitaldan merged commit 16eee55 into develop Sep 14, 2024
2 checks passed
@digitaldan digitaldan deleted the settings2swiftui branch September 14, 2024 17:54
@timbms
Copy link
Contributor Author

timbms commented Sep 14, 2024

Great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@digitaldan digitaldan digitaldan left review comments

@weakfl weakfl Awaiting requested review from weakfl

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@timbms @digitaldan