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

feat: add custom error view #1723

Merged
merged 7 commits into from
Jul 11, 2024
Merged

feat: add custom error view #1723

merged 7 commits into from
Jul 11, 2024

Conversation

d-loose
Copy link
Member

@d-loose d-loose commented Jun 28, 2024

@anasereijo we can go over design and copy for all of those next week:

Error message when refreshing a single snap or switching the channel while it is running:
Screenshot from 2024-06-27 15-41-46

Generic error page with custom message for SnapdException(kind: 'network-timeout') (see #1479 and #1668)
Screenshot from 2024-06-28 17-01-26

Generic error page with custom message for SnapdException(message: 'too many requests') (see #1684)
Screenshot from 2024-06-28 17-00-44

Ref #1479, #1668, #1684

UDENG-3056
UDENG-3264

@anasereijo anasereijo self-assigned this Jul 8, 2024
@d-loose
Copy link
Member Author

d-loose commented Jul 10, 2024

Updated error pages matching the Figma designs:

Network error:
Screenshot from 2024-07-10 17-18-48

Server error (e.g. 'too many requests')
Screenshot from 2024-07-10 17-19-14

Unknown error:
Screenshot from 2024-07-10 17-18-56

'Retry' will retry the action prior to encountering the error (loading search results, displaying a snap page, ..). 'Check status' will open https://status.snapcraft.io/ in a browser.

- add `ErrorMessage` structure for localized error title, body and
  actions
- add retry logic where possible
- link to snapcraft status page for server errors
@d-loose d-loose requested a review from spydon July 10, 2024 14:26
@d-loose d-loose marked this pull request as ready for review July 10, 2024 14:26
Copy link
Collaborator

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Lgtm functionality-wise!
I'm sure Ana will want to have a say in the design for the generic ones later. 😄

@d-loose
Copy link
Member Author

d-loose commented Jul 10, 2024

I'm sure Ana will want to have a say in the design for the generic ones later. 😄

I've double-checked Figma now and realized that I forgot to actually include the action texts for each error in the view. Added them now and updated the screenshots in my previous comment!

@anasereijo please have a quick look when you have some time :)

@Feichtmeier
Copy link
Member

Feichtmeier commented Jul 10, 2024

Imho a centered, vertical layout could be better, especially for when the window size varies

 Image
  Text 
Buttons

@spydon
Copy link
Collaborator

spydon commented Jul 10, 2024

Imho a centered, vertical layout could be better, especially for when the window size varies

 Image
  Text 
Buttons

I agree, it feels a bit "off" now, let's check that with Ana :)

@madsrh
Copy link
Member

madsrh commented Jul 10, 2024

Imho a centered, vertical layout could be better, especially for when the window size varies

+1 for centered layout 👍

I've double-checked Figma now...

Obviously, us mortals can't see the Figma mockups, but shouldn't the "headline" have a different styling? I know that it is slightly larger, but it's barely visible and doesn't feel like a propper heading. Maybe just use the same weight as we use on the Update page?

image

@anasereijo
Copy link
Collaborator

It does indeed look better centered 👍
The heading looks almost the same as the text, so if that can be changed, go for it.

The spacing between elements looks odd - the first line of text looks closer to the heading than to the second line of text. I suggest adding more padding between the heading and the text, and decrease (or remove) the padding between the two lines of text.

@d-loose
Copy link
Member Author

d-loose commented Jul 11, 2024

Column layout with adjusted spacing and font size:

Screenshot from 2024-07-11 11-46-13
Screenshot from 2024-07-11 11-45-49
Screenshot from 2024-07-11 11-44-31

@d-loose d-loose requested a review from anasereijo July 11, 2024 09:48
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.

5 participants