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

Docs: add indeterminate disabled checkbox example #36674

Merged
merged 2 commits into from Jul 6, 2022

Conversation

julien-deramond
Copy link
Member

@julien-deramond julien-deramond commented Jul 5, 2022

Based on the work of @louismaximepiton this PR proposes to add a "Disabled indeterminate checkbox" example in Forms > Check & radios > Disabled.

Live preview

@@ -102,7 +102,9 @@
// Indeterminate checkbox example in docs and StackBlitz
document.querySelectorAll('.bd-example-indeterminate [type="checkbox"]')
.forEach(checkbox => {
checkbox.indeterminate = true
if (checkbox.id.includes('Indeterminate')) {
Copy link
Member Author

Choose a reason for hiding this comment

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

The other way around would be to set checkbox.indeterminate = true only to the first example in .bd-example-indeterminate:

document.querySelectorAll('.bd-example-indeterminate')
  .forEach(example => {
    example.querySelector('[type="checkbox"]').indeterminate = true
  })

IDK what's the preferred approach here.

@julien-deramond julien-deramond marked this pull request as ready for review July 5, 2022 06:11
@julien-deramond julien-deramond requested a review from a team as a code owner July 5, 2022 06:11
@mdo
Copy link
Member

mdo commented Jul 6, 2022

Would we ever want to add a custom indeterminate attribute to use as a selector? If not, I think a class might make most sense.

@julien-deramond
Copy link
Member Author

Would we ever want to add a custom indeterminate attribute to use as a selector? If not, I think a class might make most sense.

You mean in the library itself (by having let's say as an example an .indeterminate class that will automatically launch the JS and then remove the class) or just for the documentation?

For the documentation I tried to avoid having an extra-class so that it is still understandable for the user (when copy/pasting or editing via StackBlitz) that the indeterminate state can only be set by JavaScript.

@julien-deramond julien-deramond added this to In progress in v5.3.0 via automation Jul 6, 2022
@mdo
Copy link
Member

mdo commented Jul 6, 2022

That all sounds good. I appreciate the goal of keeping the source code lean for the end user.

@julien-deramond julien-deramond removed this from In progress in v5.3.0 Jul 6, 2022
@julien-deramond julien-deramond added this to In progress in v5.2.0-stable via automation Jul 6, 2022
@julien-deramond julien-deramond moved this from In progress to Reviewer approved in v5.2.0-stable Jul 6, 2022
@julien-deramond julien-deramond merged commit 6cf7253 into main Jul 6, 2022
v5.2.0-stable automation moved this from Reviewer approved to Done Jul 6, 2022
@julien-deramond julien-deramond deleted the main-jd-indeterminate-disabled-checkbox branch July 6, 2022 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v5.2.0-stable
  
Done
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants