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

Revamp UI #9

Merged
merged 9 commits into from
Nov 24, 2024
Merged

Revamp UI #9

merged 9 commits into from
Nov 24, 2024

Conversation

Cykelero
Copy link
Collaborator

This PR completely revamps the crash reporter UI, and adds the ability for the user to provide details about the crash.

The main changes are:

  • Most elements have moved, most text has been reworded or removed
  • The crash log is now hidden behind a “Show Details” button
  • There is now a field for the user to describe how the crash happened

2024-11-17 19 00 Screenshot

@DivineDominion
Copy link
Collaborator

DivineDominion commented Nov 18, 2024

Thank you!

  • What happens when the button is clicked?
  • Did you explore the disclosure triangle option?
  • (Note to self:) The text view's font size and/or inset feels a bit on the small side. We might increase this to look more like (moden) text fields out of the box.

- Remove most constraints, use stack views instead
- Remove NoClippingView, which isn't needed when building with Sonoma SDKs and up
- Simplify the show/hide code to only hide the stack views themselves, and remove the associated outlets
The system doesn't seem to provide a large image for the warning triangle, so we're getting it from the system Diagnostics Reporter app.
- Window title and body title now mention app name
- Some of the text has been reworded, some removed
- Elements are reordered
The privacy policy button is hidden alongside the crash log. The window is now narrower.
- Fix email field placeholder not showing, and reword
- Make title text size standard
And bold the field labels.
- Updated screenshots
- Mention userProvidedDetails POST variable
@Cykelero
Copy link
Collaborator Author

I modeled the UI after the built-in macOS crash reporter; so I went along with their "Show/Hide Details" button, and the way it reveals the details inline:

That's also where I moved the privacy policy button, which didn't fit in the button row anymore.

A disclosure arrow might make more sense, but might also look less familiar, and make the window feel more cluttered because of how it would stand out.

@Cykelero Cykelero marked this pull request as ready for review November 18, 2024 13:00
@DivineDominion
Copy link
Collaborator

@Cykelero Alright, fine with me to keep the button that way then.

Does the window height setting fit small screens with zoom?

Asking because the xib shows a window height of 605 and a content view height of ~660. That should not be a problem, but some of my users send screenshots with UI scaling enabled because of poor eyesight, and only then do I notice how weird some parts can look. Windows with fixed or too large min height values can grow out of the screen bounds.

If the window would grow taller than fits a 13" MacBook at largest zoom factor, the "hide details" button could disappear out of the screen at the bottom, making it impossible to find the button to shrink the window again; it's also impossible to move the window higher up by grabbing the title bar to get to the buttons.

A more forgivingly auto-shrinking window should work without needing to rearrange buttons or add disclosure buttons.

P.S.: feel free to request a review anytime so I get a notification!

It's now 545 pixels tall at most.
@Cykelero
Copy link
Collaborator Author

That's a good point, I hadn't thought of that!

I didn't find how to cap the window height to available screen space using Auto Layout (is that possible) so I shrunk it instead; it's now 545 pixels at most. It should fit nicely in the 2020 MacBook Air's 1024×640 scaled resolution.

@DivineDominion DivineDominion merged commit 3f444d8 into CleanCocoa:master Nov 24, 2024
@DivineDominion
Copy link
Collaborator

That also works :)

I guess next it's time for me to replace Travis with GitHub Actions, and maybe check if the PHP script works with CI, too

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.

2 participants