date macro pattern #397

feedmypixel · 2017-07-31T14:05:41Z

Usage can be seen in #395 here

teneightfive · 2017-07-31T15:07:59Z

src/templates/_layouts/datahub-base.njk

@@ -11,6 +11,7 @@

 {% from "_macros/form.njk" import EntitySearchForm, Form, MultiStepForm with context %}
 {% from "_macros/form.njk" import Fieldset, TextField, MultipleChoiceField, HiddenField %}
+{% from "_macros/form.njk" import DateField %}


Could extend the line above instead:

{% from "_macros/form.njk" import Fieldset, DateField, TextField, MultipleChoiceField, HiddenField %}

teneightfive · 2017-07-31T15:09:41Z

src/templates/_macros/form.njk

@@ -351,6 +358,7 @@
 # @param {string} [props.placeholder] - Field placeholder
 # @param {string, array} [props.modifier] - form-control modifier
 # @param {string} [props.error] - Mark form-control with error
+ # @param {string} [props.maxlength] - maxlength Input attribute


Maxlength can be confusing to users. Could be better just to validate whatever the user enters rather than restricting input?

would be interesting to see what the preferred users options is. I wonder if entering something long, which disappears from view to then be told its not valid is preferred to restricting the characters entered

which disappears from view

It doesn't disappear on most browsers. The cursor will follow the last typed characters so even on smaller input fields the user will still see what they last typed. It would actually seem more like it disappears in this instance if we restrict input.

restricting the characters entered

Some users don't look at the screen when they're typing so they won't necessarily notice that everything they typed on the keyboard isn't on the screen, which is where the confusion often comes from.

This is particularly noticeable in free text fields where the accepted characters are limited. It is better to let the user type over that limit and warn them as they do so as well as validate when they submit or prevent them from submitting.

Twitter's tweet functionality is a good example - it doesn't just stop you writing, it shows you that you typed too much:

In this instance with date fields I don't think restricting input adds anything and we're better off just letting the user enter as many characters as they want to and then providing useful date validation feedback if it isn't a valid date.

great point - well made 👍

teneightfive · 2017-07-31T15:12:59Z

src/templates/_macros/form.njk

+ # @param {string} [props.inputName] - The base input name for date field names
+ # @param {string} [props.inputValue] - Field values
+ #}
+{% macro DateField(props) %}


DateFieldset?

teneightfive · 2017-07-31T15:14:22Z

assets/stylesheets/components/form/_form-control.scss

+  max-width: 4em;
+}
+
+.c-form-control--small {


Might be worth looking at @tyom's suggestion from the other PR again.

teneightfive · 2017-07-31T16:39:08Z

Might be worth adding some examples of the date fieldset to the examples in components app too.

feedmypixel · 2017-07-31T16:45:03Z

Might be worth adding some examples of the date fieldset to the examples in components app too.

👍 yeah this and tests are in the TODO's on #395

tyom · 2017-07-31T16:52:11Z

src/templates/_macros/form.njk

+ # @param {string} [props.inputName] - The base input name for date field names
+ # @param {string} [props.inputValue] - Field values
+ #}
+{% macro DateFieldset(props) %}


I try to add unit tests for all new macros. I know there' a backlog but do consider adding some tests if you can.

tyom · 2017-07-31T16:53:15Z

src/templates/_macros/form.njk

  >
    {% if props.legend %}
      <legend class="c-form-fieldset__legend">
        <span class="c-form-fieldset__legend-text">{{ props.legend }}</span>
      </legend>
    {% endif %}

+    {% if props.hint %}
+      <span id="hint-{{ props.fieldId }}" class="c-form-fieldset__hint">{{ props.hint }}</span>


Might as well use div. I doubt it would ever be displayed inline.

tyom · 2017-07-31T16:56:26Z

src/templates/_macros/form.njk

+
+  {% call FormGroup(props | assign({ fieldId: fieldId, element: 'fieldset', modifier: 'inline' })) %}
+    {{ TextField({
+      inputClass: 'c-form-control--shortest',


You can just add this to modifier modifier: ['inline', 'shortest'].

tyom · 2017-07-31T16:58:40Z

Would be good to add example of this fieldset to form components page.

tyom · 2017-08-01T18:24:22Z

Is it meant to look like this on components page?

feedmypixel · 2017-08-02T16:00:18Z

closing this as the work can be found in #395

feedmypixel · 2017-08-02T16:00:49Z

Is it meant to look like this on components page?

an error with missing stles - full PR can be seen on #395

feedmypixel · 2017-08-03T08:45:22Z

All comments addressed in #395

feedmypixel added the multipart label Jul 31, 2017

ztolley temporarily deployed to datahub-beta2-pr-397 July 31, 2017 14:05 Inactive

feedmypixel requested review from tyom, ztolley and teneightfive July 31, 2017 14:07

teneightfive reviewed Jul 31, 2017

View reviewed changes

teneightfive assigned feedmypixel Jul 31, 2017

ztolley temporarily deployed to datahub-beta2-pr-397 July 31, 2017 15:48 Inactive

feedmypixel force-pushed the feature/date-macro-pattern branch from cccce13 to 128dbaa Compare July 31, 2017 15:48

ztolley temporarily deployed to datahub-beta2-pr-397 July 31, 2017 15:48 Inactive

feedmypixel force-pushed the feature/date-macro-pattern branch from 128dbaa to 3167d28 Compare July 31, 2017 15:53

ztolley temporarily deployed to datahub-beta2-pr-397 July 31, 2017 15:53 Inactive

ztolley temporarily deployed to datahub-beta2-pr-397 July 31, 2017 16:41 Inactive

tyom reviewed Jul 31, 2017

View reviewed changes

feedmypixel added 5 commits July 31, 2017 17:52

date macro pattern

706ee74

fixup! date macro pattern

bcdd1bf

fixup! date macro pattern

3513e73

fixup! date macro pattern

89309dc

fixup! date macro pattern

3b0248c

feedmypixel force-pushed the feature/date-macro-pattern branch from 6e6a494 to 3b0248c Compare July 31, 2017 16:52

tyom reviewed Jul 31, 2017

View reviewed changes

tyom suggested changes Jul 31, 2017

View reviewed changes

fixup! date macro pattern

32af981

fixup! date macro pattern

874a00d

ztolley temporarily deployed to datahub-beta2-pr-397 August 1, 2017 08:15 Inactive

add date pattern to /components

b4313a7

ztolley deployed to datahub-beta2-pr-397 August 1, 2017 08:29 View deployment

feedmypixel closed this Aug 2, 2017

teneightfive deleted the feature/date-macro-pattern branch August 14, 2017 12:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

date macro pattern #397

date macro pattern #397

feedmypixel commented Jul 31, 2017

teneightfive Jul 31, 2017

teneightfive Jul 31, 2017

feedmypixel Jul 31, 2017

teneightfive Jul 31, 2017 •

edited

Loading

feedmypixel Jul 31, 2017

teneightfive Jul 31, 2017

teneightfive Jul 31, 2017

teneightfive commented Jul 31, 2017

feedmypixel commented Jul 31, 2017

tyom Jul 31, 2017

tyom Jul 31, 2017

tyom Jul 31, 2017

tyom commented Jul 31, 2017

tyom commented Aug 1, 2017

feedmypixel commented Aug 2, 2017

feedmypixel commented Aug 2, 2017

feedmypixel commented Aug 3, 2017

date macro pattern #397

date macro pattern #397

Conversation

feedmypixel commented Jul 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teneightfive Jul 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teneightfive commented Jul 31, 2017

feedmypixel commented Jul 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyom commented Jul 31, 2017

tyom commented Aug 1, 2017

feedmypixel commented Aug 2, 2017

feedmypixel commented Aug 2, 2017

feedmypixel commented Aug 3, 2017

teneightfive Jul 31, 2017 •

edited

Loading