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

Auth and Error View Updates #19749

Merged
merged 3 commits into from
Apr 6, 2023
Merged

Auth and Error View Updates #19749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b08d825
adds vault logo to auth page
zofskeez Mar 24, 2023
76b3934
Merge branch 'ui/VAULT-12799/sidebar-navigation' into ui/VAULT-13885/…
zofskeez Apr 4, 2023
2d99bda
updates top level error template
zofskeez Apr 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ui/app/styles/components/icon.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,11 @@
.icon-blue {
color: $blue;
}

.brand-icon-large {
width: 62px;
}

.error-icon {
width: 48px;
}
6 changes: 6 additions & 0 deletions ui/app/styles/core/helpers.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
.is-no-underline {
text-decoration: none;
}
.has-line-height-1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Want to make sure @Monkeychip is aware of these additions for the upcoming css organization!

line-height: 1;
}
.is-sideless {
box-shadow: 0 2px 0 -1px $grey-light, 0 -2px 0 -1px $grey-light;
}
Expand Down Expand Up @@ -245,6 +248,9 @@
.has-top-padding-xxl {
padding-top: $spacing-xxl;
}
.has-bottom-padding-l {
padding-bottom: $spacing-l;
}
.has-bottom-margin-xs {
margin-bottom: $spacing-xs;
}
Expand Down
5 changes: 5 additions & 0 deletions ui/app/templates/vault/cluster/auth.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
<LogoEdition aria-label="Sign in with Hashicorp Vault" />
</div>
{{else}}
<div class="is-flex-v-centered has-bottom-margin-xxl">
<div class="brand-icon-large">
<Icon @name="vault" @size="24" @stretched={{true}} />
</div>
</div>
<div class="is-flex-row">
{{#if this.mfaAuthData}}
<button type="button" class="icon-button" {{on "click" (fn (mut this.mfaAuthData) null)}}>
Expand Down
70 changes: 44 additions & 26 deletions ui/app/templates/vault/error.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,45 @@
<section class="section">
<div class="container">
{{#if (eq this.model.httpStatus 404)}}
<NotFound @model={{this.model}} />
{{else}}
<PageHeader as |p|>
<p.levelLeft>
<h1 class="title is-3 has-text-grey">
{{#if (eq this.model.httpStatus 403)}}
Not authorized
{{else}}
Error
{{/if}}
</h1>
</p.levelLeft>
</PageHeader>
<BlockError>
{{#if this.model.message}}
<p>{{this.model.message}}</p>
{{/if}}
{{#each this.model.errors as |error|}}
<p>{{error}}</p>
{{/each}}
</BlockError>
{{/if}}
<div class="is-flex-1 is-flex-v-centered">
<div class="empty-state-content">
<div class="is-flex-v-centered has-bottom-margin-xxl">
<div class="brand-icon-large">
<Icon @name="vault" @size="24" @stretched={{true}} />
</div>
</div>
<div class="is-flex-center">
<div class="error-icon">
<Icon @name="help" @size="24" class="has-text-grey" @stretched={{true}} />
</div>
<div class="has-left-margin-s">
<h1 class="is-size-4 has-text-semibold has-text-grey has-line-height-1">
{{#if (eq this.model.httpStatus 403)}}
Not authorized
{{else if (eq this.model.httpStatus 404)}}
Page not found
{{else}}
Error
{{/if}}
</h1>
<p class="has-text-grey is-size-8">Error {{this.model.httpStatus}}</p>
</div>
</div>

<p class="has-text-grey has-top-margin-m has-bottom-padding-l has-border-bottom-light">
{{#if (eq this.model.httpStatus 404)}}
Sorry, we were unable to find any content at that URL. Double check it or go back home.
{{else}}
{{this.model.message}}
{{join ". " this.model.errors}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I know sometimes we see the [object Object] error message (on the old one) -- this may be out of scope, but I wonder if a helper to process this.model.errors into something readable would help?

Copy link
Contributor Author

@zofskeez zofskeez Apr 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think that's a good suggestion @hashishaw and we should have a common way to handle API errors. We do have the error-message utility that could be used in a helper. It might not be relevant here since I'm not sure this block will actually be hit. I kept it since it was a part of the original implementation. In the cluster route there is sort of a pseudo API error being returned:

return reject({ httpStatus: 404, message: 'not found', path: params.cluster_name });

Do you think I should get rid of the other status handling and simplify the template or leave it just in case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to merge this but happy to make further changes as part of the primary PR.

{{/if}}
</p>

<div class="is-flex-between has-top-margin-s">
<ExternalLink @href="/" @sameTab={{true}} class="is-no-underline is-flex-center has-text-semibold">
<Chevron @direction="left" />
Go home
</ExternalLink>
<DocLink @path="/vault/api-docs#http-status-codes">
Learn more
</DocLink>
</div>
</div>
</section>
</div>