Skip to content

LG-568 Visual design tweak: address confirmation page#2445

Merged
donjo merged 1 commit intomasterfrom
jd-LG568-address-confirm-design
Aug 22, 2018
Merged

LG-568 Visual design tweak: address confirmation page#2445
donjo merged 1 commit intomasterfrom
jd-LG568-address-confirm-design

Conversation

@donjo
Copy link
Contributor

@donjo donjo commented Aug 21, 2018

This updates the visual design of the address verification page to reduce the size of the header and remove a phone number requirement.

screen shot 2018-08-21 at 1 10 04 pm

html: { autocomplete: 'off', method: :put, role: 'form', class: 'mt2' }) do |f|
= f.label :phone, label: t('idv.form.phone'), class: 'bold'
= f.input :phone, required: true, input_html: { class: 'us-phone' }, label: false,
= f.input :phone, required: true, input_html: { class: 'us-phone sm-col-8' }, label: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the correct place to put this class? I didn't understand where the original html was for this input form and maybe it should go there instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks right if you want to add that class to the input element. f.input generates an input inside a wrapper div. The input_html option allows you to configure the HTML output for the input. There is also a wrapper_html option you can use to configure the html for the wrapper div.

Ref: https://github.com/plataformatec/simple_form#usage

- title t('idv.titles.phone')

h1.h2.my0 = t('idv.titles.session.phone')
h1.h3.my0 = t('idv.titles.session.phone')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather see a class attached to the h2 suggesting a smaller size that select the heading level based on the visual design. But I'm not sure how much we try to follow good semantics in our HTML.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that it is an h1 and he's attached a class to it that suggests a smaller heading size. The old class list was h2 my0 and now it is h3 my0. The element itself is still an h1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup. You're right. I got stuck on the class name looking like the element name.

@jmhooper
Copy link
Contributor

Looks like you got stuck on a flickering spec in CI. I've restarted CI and I'll open a PR to fix that spec in a couple minutes.

@donjo donjo merged commit 30e2188 into master Aug 22, 2018
@donjo donjo deleted the jd-LG568-address-confirm-design branch August 22, 2018 16:04
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