search

XcodesOrg / XcodesApp

The easiest way to install and switch between multiple versions of Xcode - with a mouse click.
MIT License
6.75k stars 294 forks source link

Clean up code in `InfoPane` to be more robust #436

Closed thai-d-v closed 9 months ago

thai-d-v commented 10 months ago

Problem of InfoPane

  1. The preview code is too heavy. It crashes all the time.
  2. Explicit dependencies are too broad
    • make it hard to create preview data (in reality)
    • reduce performance (in theory)
    • reduce reusability (in the future)
  3. The body is too big
    • hard to reason
    • hard to test by preview

Approach

  1. I move subviews to separate files

    • reduce data dependency
    • reduce logic responsibilities, focus mostly on displaying data
    • reduce layout responsibilities. Let the outer view decide the layout
    • add preview to test locally

  2. I fix InforPane

    • reduce data dependency
    • improve preview performance. It is fast and doesn't crash anymore
    • fix layout
    • I move UnselectedView out to:
      • InfoPane should only care about displaying data, not whether an Xcode is selected

Discussion

MattKiazyk commented 9 months ago

Hi @thai-d-v

First of all thank you for taking the time to make Xcodes better!

I'm all for making InfoPane easier to read, easier to preview and all you've said above. This is a really good start.

The main question I have is around all the WrapperViews? What is the intent of those? I would rather see previews using the main view it's previewing vs having a wrapper.

Thanks again - let me know how I can help

thai-d-v commented 9 months ago

Hi @MattKiazyk, WapperView provides a local interactive preview for InfoPane. It has some main benefits:

  1. faster preview
  2. local reasoning: we can preview interactively many states and layout of InfoPane with a small dependency
MattKiazyk commented 9 months ago

@thai-d-v thanks for the response. I see some benefit in it, however I would rather stick with the Apple way of using different previews for each view you are previewing. Added benefit would be that the view you are working on can be previewed as you are working on. In the WrapperView scenario, I would have to change the state PreviewName to be the one I'm on every time I wanted to edit and preview.

Feel free to use the #Preview macro now if you rebase from main. CI is now Xcode 15.

thai-d-v commented 9 months ago

@MattKiazyk You're right, code looks much cleaner with #Preview. I updated it.

shakeliaj commented 9 months ago

Can you please remove me from this mailing list????

On Thu, Nov 23, 2023 at 12:35 PM Thai D. V. @.***> wrote:

@MattKiazyk https://github.com/MattKiazyk You're right, code looks much cleaner with #Preview. I updated it.

— Reply to this email directly, view it on GitHub https://github.com/XcodesOrg/XcodesApp/pull/436#issuecomment-1824764967, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE2MTBFGJID6HI3Z642YOD3YF6COFAVCNFSM6AAAAAA6AD7U7WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRUG43DIOJWG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>