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

Add kit version to footer #476

Merged
merged 3 commits into from
Aug 15, 2018
Merged

Add kit version to footer #476

merged 3 commits into from
Aug 15, 2018

Conversation

mikeshawdev
Copy link
Contributor

@mikeshawdev mikeshawdev commented Jan 9, 2018

This change adds a line to the footer, which states the current version of the prototyping kit being used. Screenshot below.

footer-prototype-version

This makes it clear to users which version of the kit is being used, as they don't need to check the console or source code for this information when using a prototype.

Resolves #455.

@edwardhorsford
Copy link
Contributor

@mikeshawdev thanks for this!

Would you be able to amend the description so it's clearer what this changes and why you think it's good to do? it would also be great to include a screenshot of how it appears on pages.

@joelanman
Copy link
Contributor

@mikeshawdev sorry for the delay, thanks for this work, it's definitely something that would helpful. Do you have time to update or respond to the code comments?

@mikeshawdev
Copy link
Contributor Author

@joelanman no worries, I've updated the description of the PR in response to @edwardhorsford's comments

@joelanman
Copy link
Contributor

@mikeshawdev oh geez my mistake, I made a review but didn't 'submit' it so you can't see it! Sorry about that, submitting now

@@ -36,14 +36,21 @@
with-proposition
{% endblock %}

{% block footer_top %}
{# <p>Prototype kit version {{ releaseVersion }}</p> #}
{% endblock %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this work! Is this section supposed to be here? It looks like it's just got a commented-out line in it

<ul>
<ul>
<li>
Prototype kit {{ releaseVersion }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be GOV.UK Prototype Kit - makes it consistent with how we refer to it elsewhere

Signed-off-by: Mike Shaw <mike.shaw@dwp.gsi.gov.uk>
@mikeshawdev
Copy link
Contributor Author

@joelanman updated the code based on your comments 👍

@joelanman
Copy link
Contributor

Thanks! Can you add a line to CHANGELOG?

Signed-off-by: Mike Shaw <mike.shaw@dwp.gsi.gov.uk>
@mikeshawdev
Copy link
Contributor Author

Done, and done!

@joelanman
Copy link
Contributor

Hi @mikeshawdev really sorry, just had a quick discussion on the team and can we go back to it not being a link? Not sure of the user need, and not 100% sure that the generated link will always work (the tag in GitHub would have to be exactly the same as the version in package.json, which it ought to be, but they could possibly go out of sync)

@mikeshawdev
Copy link
Contributor Author

mikeshawdev commented Aug 15, 2018

@joelanman great minds think alike! I've just opened this issue on govuk-frontend to allow plain text within the govukFooter macro: alphagov/govuk-frontend#956

As it stands, you cannot have plain text in that list so I've had to use a link for now

@joelanman
Copy link
Contributor

Sorry, telling you to do something that can't be done! In the meantime, I think maybe change the link to just go to the Prototype Kit site: https://govuk-prototype-kit.herokuapp.com/

Signed-off-by: Mike Shaw <mike.shaw@dwp.gsi.gov.uk>
@mikeshawdev
Copy link
Contributor Author

Amend made 👍

@joelanman
Copy link
Contributor

great! thanks

@joelanman joelanman merged commit 6a282f9 into alphagov:master Aug 15, 2018
@mikeshawdev mikeshawdev deleted the feature/add-kit-version-to-footer branch August 15, 2018 10:48
@joelanman joelanman mentioned this pull request Aug 15, 2018
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.

3 participants