-
Notifications
You must be signed in to change notification settings - Fork 10
Implement new input types #34
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
base: master
Are you sure you want to change the base?
Conversation
|
@tesk9 would be happy for some early stage feedback on this one if you get some time in the next while 👍🏻! |
e7ca1f1 to
87bfb2d
Compare
tesk9
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I've only read up through inputColor -- out of time for now! I'll come back to this)
| Use the HTML autocomplete attribute whenever possible. Read [Understanding Success Criterion 1.3.5: Identify Input Purpose](https://www.w3.org/WAI/WCAG21/Understanding/identify-input-purpose) and [Using HTML 5.2 autocomplete attributes (Technique H98)](https://www.w3.org/WAI/WCAG21/Techniques/html/H98) for more information. | ||
|
|
||
| You might notice that `Html.Attributes` and `Html.Attributes` don't provide full autocomplete support. This is tracked in [elm/html issue 189](https://github.com/elm/html/issues/189). | ||
| You might notice that `Html.Attributes` doesn't provide full autocomplete support. This is tracked in [elm/html issue 189](https://github.com/elm/html/issues/189). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! This is what comes of having accessible-html and accessible-html-with-css, and trying to keep them in line with each other 😅
35fea13 to
1a44044
Compare
f430110 to
1ea1cb7
Compare
elm.json
Outdated
| }, | ||
| "test-dependencies": { | ||
| "elm-explorations/test": "1.2.1 <= v < 2.0.0" | ||
| "elm-explorations/test": "2.0.0 <= v < 3.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated due to failing local tests and also noticed in PR #37 that CI fails with this versioning issue also.
|
|
||
| {-| -} | ||
| type alias Attribute msg = | ||
| Html.Attribute msg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really sure why these were implemented, I am sure there was a reason but it seems unnecessary since they mirror the base implementations. Since we have a major release either way with #35 and this PR, I felt safe in removing these and using the base implementations accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So these exist so that instead of:
import Accessibility exposing (..)
import Html exposing (Attribute)
you can instead do:
import Accessibility exposing (..)
How useful this is probably varies from project to project (and import style to import style), but that's the intent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I re-exported the Attribute and Html helpers then.
|
|
||
| -} | ||
| empty : Html msg | ||
| empty = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty nodes can be useful in cases such as in scenarios like viewJust where you have something like:
viewJust : (a -> Html msg) -> Maybe a -> Html msg
viewJust fn v =
Maybe.map fn v |> Maybe.withDefault emptyThat is just one example but I have seen this kind of thing in use and personally regularly use it also and so exposing it from here as a helper could be nice but I can remove it if you'd prefer it wasn't around, just a proposal.
|
Resolved conflicts with latest |
|
Conflicts are now resolved. |
Implement new input types with compliant validations in place and test coverage for simple happy path and fallback cases. Implementing inputs as per MDN specs.