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

Match capy click selectors with underlying html #1856

Merged
merged 4 commits into from
Oct 26, 2023

Conversation

rosschapman
Copy link
Contributor

@rosschapman rosschapman commented Sep 28, 2023

Regarding #1848 (comment)

The difference between buttons and links are syntactically and semantically confusing. I would love to better understand the accessibility standards around this. Having some expertise here would be helpful. But until we dig into that, what I was thinking is that we could force the developer into a syntactically correct choice to remove any ambiguity in the code about whether an element is a <button> or <a>.

@rosschapman rosschapman force-pushed the match-click-selectors-with-html branch from 8631c39 to f19662e Compare September 28, 2023 00:55
@rosschapman rosschapman changed the title Bump rubocop-rspec from 2.24.0 to 2.24.1 Match capy click selectors with underlying html Sep 28, 2023
@rosschapman rosschapman marked this pull request as ready for review September 28, 2023 00:59
@rosschapman rosschapman requested review from zspencer, anaulin, KellyAH and a team September 28, 2023 00:59
Copy link
Member

@anaulin anaulin left a comment

Choose a reason for hiding this comment

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

I think this is a good change, thank you!

You're going to have to fix a bunch more Capybara-based specs for this to go green tho.

@anaulin
Copy link
Member

anaulin commented Sep 28, 2023

You can add exceptions for the newly-failing files if you want, and check this in. That way you are making some progress, but don't need to sink a bunch of time into fixing all these other occurrences.

@zspencer
Copy link
Member

Yea, we have some work to do to clean up how we're using links and buttons. It was my second project using ViewComponent and Turbo; and I basically brute-forced things to work.

But getting to the point where buttons == change data and display and links == change display will be a huge win. Probably not the most urgent project but something to look out for as we make other changes.

@rosschapman rosschapman force-pushed the match-click-selectors-with-html branch from d6c4f9e to 0f1ab2a Compare October 26, 2023 00:34
@rosschapman
Copy link
Contributor Author

@anaulin @zspencer thanks for the review and notes. Only took me ~10 mins to cleanup the rest of the style check failures. 🚀

@rosschapman rosschapman merged commit 9c4568b into main Oct 26, 2023
4 checks passed
@rosschapman rosschapman deleted the match-click-selectors-with-html branch October 26, 2023 00:42
@zspencer
Copy link
Member

Wooo! Thank you!

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