Skip to content

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.

3 participants