-
Notifications
You must be signed in to change notification settings - Fork 8
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
Standardise element ID hook attribute #21
Comments
Thanks for the issue and especially for the thorough examples. I like the idea of making it configurable so people can use whatever they like! I'm not sold on I think by default though, the ambiguity makes things less clear. Allowing you to configure things if you have a pattern you like, or that another tool supports allows the best of both worlds. Also, even though technically you must add I'm ok leaving it as Maybe I feel this is a good balance between letting people choose and having a clear and succinct default. What do you think? |
Thanks for the thoughtful response. I like brevity and nice looking code too. If "invalid" custom attrs work fine and are already widely used then that's a pretty strong argument. I also like reliable code and minimising risk of future issues such as name collisions. But really a collision could happen either way, it's hard to predict what combinations of libs will be used.
My takeaway would be I don't see a clear "best" default, they're all good to me. Configurability would allow personal preference and fixing collisions with specific combinations of libraries. Perhaps stick with |
Ooh this is a nice article on the issue of using IDs for stuff other than styling. They also went with |
After spending some time in Flow, I'd actually like to propose getting rid of
I think one argument that article makes against using On the plus side, the one thing I do like about them is that if you move that element somewhere else on the page, your specs would still pass if written correctly. Aside from that, I find myself wanting to use them less. |
I like that Flow encourages using a custom attribute (
flow-id
) rather than regularid
for selecting elements, as it decouples logical selection from CSS styling selection, thus should be more flexible and less brittle. Butflow-id
seems quite project specific as an attribute name (and invalid HTML markup).config.hook_attribute = "flow-id"
.data-
to be valid markup.Unless the HTML spec has changed to allow this, defining a custom attribute like
flow-id
is invalid markup. HTML5 provides data attributes as a valid way to do this. Angular naughtily supportsng-*
attributes thought they also allow the validdata-ng-*
form. There is obviously a brevity tradeoff, but I feel a framework should encourage the best practice, while optionally allowing alternate configuration to taste.Adding an "ID-like" attribute for programatically selecting elements (rather than stylistically) seems like a common thing to do. You might do it in JS for logic. You might do it in other spec frameworks. One other example I know is Deface; a Ruby tool for programatically rewriting views. Deface doesn't have or enforce any custom-ID shortcut but Spree (which uses Deface heavily) has settled on
data-hook
as the preferred way of selecting elements. Perhaps LuckyFlow could usedata-hook
by default? Are there other tools that use a differentdata-*
attribute for similar purpose?FWIW I'm starting to use
data-hook
in my Rails Rspec Capybara specs.The text was updated successfully, but these errors were encountered: