-
Notifications
You must be signed in to change notification settings - Fork 5
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/1571 forgotten username display username #6450
Feat/1571 forgotten username display username #6450
Conversation
|
||
ngOnInit(): void { | ||
this.getUsernameFound(); | ||
this.isUsernameFound = this.username !== null ? true : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably we could simplify it as
this.isUsernameFound = this.username !== null ? true : false; | |
this.isUsernameFound = this.username !== null |
Or, at the html template, we could set the if condition as username !== null
class="govuk-panel govuk-panel--gray govuk-util__align-left govuk-!-margin-top-1 govuk-!-padding-top-1 govuk-!-padding-bottom-5 govuk-!-padding-left-2 govuk-!-padding-right-0 panel" | ||
data-testid="panel" | ||
> | ||
<h1 class="govuk-!-font-size-36 govuk-!-margin-top-4 govuk-!-margin-bottom-3"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use govuk-heading-l
instead of govuk-!-font-size-36
here. Both are same font size, but in most case we use the heading classes for h1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to use the gov heading class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, gave a few small suggestions
Work done
Tests
Does this PR include tests for the changes introduced?