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

Style buttons as selected on <details open> #346

Merged
merged 2 commits into from
Sep 22, 2017

Conversation

muan
Copy link
Contributor

@muan muan commented Sep 13, 2017

We are trying to convert more and more core function dropdowns on the site to using <details>. I'm currently working on https://github.com/github/github/pull/78190 and we need the selected state on <summary class="btn"> without the .selected class being added since the <details> pattern uses no JavaScript.

I'm curious what you all think about this, and if there is a better solution. ⭐ Happy to explore.

/cc @primer/ds-core

@shawnbot
Copy link
Contributor

This gets a big 👍 from me.

Slightly off-topic of this PR, but I'm not quite sure where else to put it: Should we consider adding either <details> reset styles to primer-base or a utility class to reset them? I ask because @katmeister and I did some hacking in https://github.com/github/github/pull/77914/commits/8a9ab7f799492378bd9de2d6e5a6fd2be57956c0 that includes a reset snippet from one or your refactors, and I think it'd make it a lot easier to deprecate the details component/behavior in favor of the HTML5 elements.

That commit also adopts an approach that @keithamus suggested, minting some new classes: details-content--shown and --hidden vs. Details-content--shown. Maybe it's better to name them details-content--open and details-content--closed to be clearer, but what do you all think about that approach?

@muan muan requested a review from a team September 15, 2017 07:49
@muan
Copy link
Contributor Author

muan commented Sep 15, 2017

@shawnbot thanks! I commented on that PR about the reset styles.

Copy link
Contributor

@shawnbot shawnbot left a comment

Choose a reason for hiding this comment

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

This is great. It means that folks can prototype with <details> and no JS! Let's get this included in 9.5.0 ASAP.

@shawnbot shawnbot changed the base branch from dev to release-9.5.0 September 22, 2017 21:24
@shawnbot shawnbot merged commit 27762e3 into release-9.5.0 Sep 22, 2017
@shawnbot shawnbot deleted the muan/details-open branch September 22, 2017 21:25
@shawnbot shawnbot mentioned this pull request Sep 22, 2017
4 tasks
@muan muan mentioned this pull request Oct 16, 2017
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.

2 participants