GOV.UK Framework | Workplace | Confirm Vacancies #696
Conversation
Adding definition list CSS to lists.scss
| <p class="govuk-heading-m">You told us you have:</p> | ||
| <dl class="govuk-list--definition govuk-list--definition-xl govuk-!-margin-bottom-8"> | ||
| <dt>Current staff vacancies</dt> | ||
| <dd>{{ establishment.TotalVacencies }}</dd> |
There was a problem hiding this comment.
I assume TotalVacencies property is coming back from the BE like this? weird casing and spelling...
| background-color: govuk-colour('blue'); | ||
| text-align: left; | ||
|
|
||
| * { |
There was a problem hiding this comment.
is the wildcard selector needed here? if so do we need the color's applied to the anchor states?
There was a problem hiding this comment.
yeah, if the wildcard isn't there we'd have to set the colour for every element as it's usually the default black.
There was a problem hiding this comment.
I think this is going to be a temporary fix until they bring in the Interruption Card properly in GDS alphagov/govuk-design-system-backlog#27
| color: #fff; | ||
| } | ||
|
|
||
| a { |
There was a problem hiding this comment.
could probs combine like
a, a:link, a:visited { color: #fff; )
also would be good to use variables for colors src/assets/scss/partials/_colors.scss
There was a problem hiding this comment.
We should probably extend the govuk-colour map to contain these new colours and then use that instead of just the colour variable, will update though
There was a problem hiding this comment.
Colours updated and rules grouped
| .pipe(map(jobs => jobs.TotalVacencies)) | ||
| .subscribe(total => (this.total = total)) | ||
| this.subscriptions.add( | ||
| this.establishmentService.establishment$.pipe(take(1)).subscribe(establishment => { |
There was a problem hiding this comment.
do we need to subscribe to to this observable? the workplace module has the Auth guard set so to be able to access this route the user has to be logged in and therefore the establishment would be already set on the EstablishmentService anyway? we could probs just do something like this.establishment = this.establishmentService.establishment;....?
There was a problem hiding this comment.
After taking another look at this we should leave it in. Comes down to the state management RXJS Pattern and best practices, you should access the state directly, and instead subscribe to the observable. This would definitely be clearer with NGXS/NGRX as there's a set way to pull data from the state. I do the same thing on the Question pages, which this is a part of.
There was a problem hiding this comment.
Btw, there isn't any extra HTTP request that is being made for this page.
| ); | ||
| } | ||
|
|
||
| ngOnDestroy() { | ||
| this.subscriptions.forEach(s => s.unsubscribe()); | ||
| this.subscriptions.unsubscribe(); |
There was a problem hiding this comment.
I dont think you need to unsubscribe if you do a take(1)
There was a problem hiding this comment.
The reason I've put the unsubscribe here is if the person navigates away from the page we want to cancel that request, very unlikely, but just good practice I think. And per your previous comment I think this can probably be removed too.
Extended the Panel SCSS to include an interruption card alphagov/govuk-design-system-backlog#27
Added Definition List SCSS to display block of 3 list items, using CSS Grids, making sure it's prefixed for IE.
Updating Confirm Vacancies to use the GOV.UK Design Framework