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

WIP: Updating date component to be type text and allow inputmode #1599

Closed
wants to merge 1 commit into from
Closed

WIP: Updating date component to be type text and allow inputmode #1599

wants to merge 1 commit into from

Conversation

htmlandbacon
Copy link
Contributor

@htmlandbacon htmlandbacon commented Oct 2, 2019

Opening for discussion, following on from pull request 1527 - I've applied this to the date component.

Before I go too far I figured there is a couple that probably needs talking through.

  • I've allowed type and inputmode to be overridden - should this be the case?

  • Given we recommend not using html5 validation, should we remove the pattern params?

  • We should update the example with maxlength attribute?

My thoughts where no, yes and yes.

Happy to take onboard any addtional comments 👍

I've done manual testing in on Browserstack and had a Dragon user test it.

Following on from #1449 (comment)

@htmlandbacon htmlandbacon changed the title WIP: Updating date component to be type text and allow input mode WIP: Updating date component to be type text and allow inputmode Oct 2, 2019
@NickColley
Copy link
Contributor

NickColley commented Oct 7, 2019

Heya Colin,

I've allowed type and inputmode to be overridden - should this be the case?

I think unless you have a very clear reason why someone wants to do this, we should not allow them to be overridden for now as it expands the API surface area without a need. If we then get a clear need to override this in the future we can add this functionality without a breaking change.

Given we recommend not using html5 validation, should we remove the pattern params?

We should consider this but it would be a breaking change so would need to be against the 4.0.0 milestone, so if we want to do this we should raise a new issue to track removing that.

We should update the example with maxlength attribute?

Sorry if I've missed some important context but we typically avoid using this attribute as it is less usable, copy pasting, typing without looking, autofill etc.

I've done manual testing in on Browserstack and had a Dragon user test it.

Nice! We're the results positive?

@htmlandbacon
Copy link
Contributor Author

Thanks for the info, I'll make them changes, and work out what else to raise!

In terms of the below, is this documented any where? Be a great thing to point people to if so!

Sorry if I've missed some important context but we typically avoid using this attribute as it is less usable, copy pasting, typing without looking, autofill etc.

Also yes, which shouldn't be a surprise since it has wide support https://caniuse.com/#search=inputmode

@NickColley NickColley added the awaiting triage Needs triaging by team label Oct 8, 2019
@NickColley
Copy link
Contributor

I was talking about you comment on maxlength .

We should update the example with maxlength attribute?

I'm not sure what you're referring by this actually, I think I got confused haha.

It looks like to ship this we might want to do some work alongside it as noted in the original comment: #1449 (comment)

So I've put awaiting triage on this for us to chat as a team about how we go about doing that.

@amyhupe amyhupe added documentation User requests new documentation or improvements to existing documentation Effort: days and removed awaiting triage Needs triaging by team labels Oct 16, 2019
@hannalaakso
Copy link
Member

hannalaakso commented Oct 23, 2019

@htmlandbacon Thanks for your patience. There are some accompanying actions/guidance we want to do alongside this work so we're not able to merge this yet. We're going to look at this in a few weeks' time when we'll be doing accessibility fixes.

@edwardhorsford
Copy link
Contributor

Given we recommend not using html5 validation, should we remove the pattern params?

I might be wrong, but I think this is here so iOS displays a numeric keypad.

@36degrees
Copy link
Contributor

I might be wrong, but I think this is here so iOS displays a numeric keypad.

Just to clarify, in iOS 12.2 and above inputmode dictates the keyboard used. I believe pattern can still be used, but has no effect if inputmode is set.

IMO we should base our decision on dropping pattern on the amount of traffic we're seeing from devices running iOS < 12.2.

@NickColley
Copy link
Contributor

Hey Colin,

Just an update: we were hoping to push forwards with this soon but have had to re-prioritise some stuff to focus on making our contribution and support more sustainable, so we may not get back to this for a while longer.

Thanks for your patience 👍

@htmlandbacon
Copy link
Contributor Author

Hey Colin,

Just an update: we were hoping to push forwards with this soon but have had to re-prioritise some stuff to focus on making our contribution and support more sustainable, so we may not get back to this for a while longer.

Thanks for your patience 👍

I've been away so no issue. 😂

@36degrees
Copy link
Contributor

I've allowed type and inputmode to be overridden - should this be the case?

I think unless you have a very clear reason why someone wants to do this, we should not allow them to be overridden for now as it expands the API surface area without a need. If we then get a clear need to override this in the future we can add this functionality without a breaking change.

Agreed, can we remove this?

Given we recommend not using html5 validation, should we remove the pattern params?

We should consider this but it would be a breaking change so would need to be against the 4.0.0 milestone, so if we want to do this we should raise a new issue to track removing that.

I've raised #1701 to address this.

@htmlandbacon we're looking to pick this up this sprint – would you have time to remove the ability to customise the input type and inputmode, or shall we take that on? Thanks again for your patience with this one.

@36degrees
Copy link
Contributor

@htmlandbacon thanks very much for your work on this so far – we can pick this up and take it forward.

@htmlandbacon
Copy link
Contributor Author

@htmlandbacon thanks very much for your work on this so far – we can pick this up and take it forward.

No worries, apologies for not replying - I was trying to work out if I could progress it this week!

Let me know if there is anything I can support on 🎈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation User requests new documentation or improvements to existing documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants