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

Improve role date selection #1658

Merged
merged 2 commits into from Aug 13, 2014
Merged

Improve role date selection #1658

merged 2 commits into from Aug 13, 2014

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Aug 13, 2014

  • Starting from 1700 and having to scroll all the way down to 2014, or even 2019, is painful
  • Restrict date entries to current year rather than current year + 5 which seemed to be the default
  • Refine copy about why roles cannot be changed later
* Starting from 1700 and having to scroll all the way down to 2014, or
even 2019, is painful
* Restrict date entries to current year rather than current year + 5
which seemed to be the default
@fofr

This comment has been minimized.

Copy link
Contributor Author

@fofr fofr commented on 5bcbc56 Aug 13, 2014

Before:
screen shot 2014-08-13 at 10 24 10

After:
screen shot 2014-08-13 at 10 24 35

This comment has been minimized.

Copy link

@wryobservations wryobservations replied Aug 13, 2014

@wryobservations
Copy link

@wryobservations wryobservations commented Aug 13, 2014

Looks good to me.

Lisa Scott
Associate Product Manager, GOV.UK
Government Digital Service, Aviation House, 125 Kingsway, London WC2B 6NH

lisa.scott@digital.cabinet-office.gov.uk

Product updates for GOV.UK: https://insidegovuk.blog.gov.uk/
GDS blog: http://digital.cabinetoffice.gov.uk/

Please use the support form! https://www.gov.uk/support/internal

On 13 Aug 2014, at 13:37, Paul Hayes notifications@github.com wrote:

Starting from 1700 and having to scroll all the way down to 2014, or even 2019, is painful
Restrict date entries to current year rather than current year + 5 which seemed to be the default
Refine copy about why roles cannot be changed later
You can merge this Pull Request by running

git pull https://github.com/alphagov/whitehall improve-role-date-selection
Or view, comment on, or merge it at:

#1658

Commit Summary

Start role year selects at this year, not 1700
Refine role copy
File Changes

M app/views/admin/role_appointments/_form.html.erb (6)
Patch Links:

https://github.com/alphagov/whitehall/pull/1658.patch
https://github.com/alphagov/whitehall/pull/1658.diff

Reply to this email directly or view it on GitHub.

@@ -9,15 +9,15 @@
disabled: !form.object.new_record? %>

<% if form.object.new_record? %>
<p>Once an appointment has been created the person cannot be changed, to preserve this person's relationship to any associated speeches.</p>
<p>Once an appointment has been created the person cannot be changed, this is to preserve the persons relationship to associated speeches.</p>

This comment has been minimized.

@dhwthompson

dhwthompson Aug 13, 2014
Contributor

I’m not sure this sentence still holds together with just a comma. I’m no content designer, but could I suggest using a semicolon instead?

This comment has been minimized.

@alext

alext Aug 13, 2014
Contributor

Or even just 2 sentences?

This comment has been minimized.

@tekin

tekin Aug 13, 2014
Contributor

Agreed. Full stop instead of comma.

This comment has been minimized.

@fofr

fofr Aug 13, 2014
Author Contributor

I agree. Two sentences. Rebased.

* The second part of the sentence was hard to understand
@tekin
Copy link
Contributor

@tekin tekin commented Aug 13, 2014

Code looks good 👍. Merge away once PO-accepted.

fofr added a commit that referenced this pull request Aug 13, 2014
Improve role date selection
@fofr fofr merged commit f8fcafe into master Aug 13, 2014
1 check passed
1 check passed
default Build #3804 succeeded on Jenkins
Details
@fofr fofr deleted the improve-role-date-selection branch Aug 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.