getditto / DittoSwiftTools

Diagnostic and Debugging Tools for DittoSwift
MIT License
9 stars 2 forks source link

Refactor Initial Screen and Enhance View Encapsulation #152

Closed rdas-ditto closed 2 months ago

rdas-ditto commented 2 months ago

Overview:

This PR simplifies the app's initial screen, and improves view encapsulation and readability.

Key Changes:

  1. Ditto Tools App: Removed Redundant Initial Screen:

    • Updated the root view to display AllToolsMenu directly on the initial screen.
    • Added a toolbar item for displaying the login sheet.
    • Removed unused @State variables and added a sidebar menu for iPad.
  2. Improved Encapsulation:

    • Cleaned up imports and separated tvOS-specific code for clarity.
    • Added SDK version as a toolbar item (iOS) that copies to clipboard on tap.
    • Renamed sections:
      • “Debug” → “Diagnostics”
      • “Exports” → “Data Exporting”
    • Refactored “Export Data…” to a button and encapsulated logic for alerts and sheets into the ExportButton.

Impact:

Testing:

Screenshots:

iOS:

Frame 1

iPadOS:

Frame 2

tvOS:

Frame 3

bplattenburg commented 2 months ago

Looks really clean!

Do we lose the Change Identity UI here? That's not part of the AllToolsMenu tool because its only used in the test app, tool usage in a customer app would have an existing Ditto Identity.

rdas-ditto commented 2 months ago

Thanks!

The Change Identity UI has just been moved to a toolbar item (a gear icon in the top right corner), which in fact toggles .isShowingLoginSheet in the viewModel instead of using a navigation item.

That implementation still resides in the App portion, rather than the framework, as it remains up to Customers to implement their own identity management.

bplattenburg commented 2 months ago

That will teach me not to skim the description and screenshots instead of diving deeper into the code.

Thanks for taking the extra time to ensure this works well on iPhone, iPad, and tvOS!

rdas-ditto commented 2 months ago

Pulled changes from Main, and verified it runs on Mac too.

Frame 7

rdas-ditto commented 2 months ago

I was holding out to check if my changes were working on Apple TV hardware, but discovered the issue I was seeing is present on main, so I'm going to go ahead and close/merge this and I've opened an issue.