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

Upgrade to Django 1.11, use uswds_forms. #1640

Merged
merged 9 commits into from Mar 21, 2018

Conversation

Projects
None yet
2 participants
@toolness
Contributor

toolness commented Mar 21, 2018

This upgrades us to Django 1.11 and also moves us to using django-uswds-forms, since Django 1.11 breaks our custom widgets.

This PR supersedes #1549.

Notes

  • We override uswds-forms' date template so we can use our own <uswds-date> custom element. Our override is largely a copy-paste of the original with a few alterations.

  • Our date widget uses USWDS classes that aren't prefixed with usa-, which is weird. It's fine, since in order to use <uswds-date> I had to provide a custom template anyways, but it's still weird. I think there might be a reason behind this that I'm simply not remembering... Ohhh, I guess it's because of this.

  • The input fields in our date widget used to use the input-inline class, and now they use the usa-input-inline class, but this seems to make no visual difference because we didn't have any CSS rules for input-inline to begin with (and we don't have any for usa-input-inline either). Since switching to input-inline would take extra work, I decided not to pursue it, as it's apparently meaningless.

To do

We can potentially punt some of these to issues.

  • Fix display errors w/ dates.
  • Get rid of any templates that are no longer used.
  • Ensure dates are using our <uswds-date> custom element.
  • Get rid of frontend/widgets.py.
  • Get rid of frontend/date.py.
  • Add tests to ensure our custom date template works OK.
  • Update the style guide as needed, so that it doesn't refer to non-existent widgets.
  • Manually test stuff and make sure it all works okay.
  • Consider getting rid of some of our CSS that makes Django's default classes--errorlist, helptext, etc--look like USWDS. (This is because django-uswds-forms is able to tell Django 1.11 to use the official USWDS classes instead.) Moved to #1646.
  • Consider replacing our custom {% fieldset %} template tag with {% fieldset %} from uswds-forms. Moved to #1646.
  • Consider using uswds_forms.UswdsForm and/or uswds_forms.UswdsErrorList, if they'd simplify our codebase. Moved to #1646.

@toolness toolness requested a review from jseppi Mar 21, 2018

@toolness

This comment has been minimized.

Contributor

toolness commented Mar 21, 2018

@jseppi I think this is good to go (I'd like to split out the last unfinished tasks into separate tickets, if that's ok).

@toolness toolness changed the title from [WIP] Upgrade to Django 1.11, use uswds_forms. to Upgrade to Django 1.11, use uswds_forms. Mar 21, 2018

@jseppi

This comment has been minimized.

Contributor

jseppi commented Mar 21, 2018

Yup, definitely ok to split out to separate task!

@jseppi

This comment has been minimized.

Contributor

jseppi commented Mar 21, 2018

Minor thing: on the date widgets, the hint text used to have a slight gray color to it.

See screenshot - left is this PR, right is current styling:

screen shot 2018-03-21 at 9 41 27 am

@toolness

This comment has been minimized.

Contributor

toolness commented Mar 21, 2018

Oh good catch, just fixed that!

@jseppi

jseppi approved these changes Mar 21, 2018

This looks 💯 to me!

@toolness

This comment has been minimized.

Contributor

toolness commented Mar 21, 2018

Yay thanks! We are now good to go with 1.11 until at least April 2020!

@toolness toolness merged commit 316d6d2 into develop Mar 21, 2018

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codeclimate All good!
Details

@toolness toolness deleted the django-1-11 branch Mar 21, 2018

@toolness toolness referenced this pull request Mar 21, 2018

Closed

Use uswds_forms for more things #1646

0 of 3 tasks complete

@toolness toolness referenced this pull request Mar 26, 2018

Closed

Upgrade to Django 1.11 #1346

1 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment