A11-2660 refactor and add ClickableText tests #1561
Conversation
charbelrami
requested review from
a team and
bcardiff
and removed request for
a team
tests/Spec/Nri/Ui/ClickableText.elm
Outdated
| @@ -1,72 +1,411 @@ | |||
| module Spec.Nri.Ui.ClickableText exposing (spec) | |||
| module Spec.Nri.Ui.ClickableText exposing (..) | |||
There was a problem hiding this comment.
Is there a reason to expose everything instead of the top level spec?
There was a problem hiding this comment.
that was unintended
tests/Spec/Nri/Ui/ClickableText.elm
Outdated
| helpfullyDisabledClickableText : List Test | ||
| helpfullyDisabledClickableText = | ||
| [ test "does not have `aria-disabled=\"true\" when not disabled" <| | ||
| type Type_ |
There was a problem hiding this comment.
Nothing against this Type_, but from I can see having programLink and programButton functions would be enough today. Are there any plans in the future?
There was a problem hiding this comment.
no plans! Updated!
tests/Spec/Nri/Ui/ClickableText.elm
Outdated
|
|
||
| testAsButton : Test | ||
| testAsButton = | ||
| test "the `button` type renders as a button element" <| |
There was a problem hiding this comment.
I think we usually nest describes and tests directly. Is there a reason to flatten the specs and organizing them as proposed?
I checked on our elm style guide and there is nothing against but it seems like a deviation from what we usually do, so I wonder the motivations in case I am missing something.
There was a problem hiding this comment.
no specific reason, I just thought it would look cleaner, but don't feel strongly. Updated!
|
thank you @bcardiff ! |
Restructures the ClickableText spec, improves the descriptions for better understanding, adds tests.