Skip to content

Conversation

@muan
Copy link
Contributor

@muan muan commented May 23, 2018

This moves <details> utility classes we use on dotcom to Primer. Added docs & stories. Hopefully I did them correctly!

/cc @primer/ds-core

@muan muan requested review from a team and jonrohan May 23, 2018 20:56
@shawnbot
Copy link
Contributor

I've been confused by the "expanded" qualifier in the past. I wonder if there's a better word to describe it, like "modal"?

@muan
Copy link
Contributor Author

muan commented May 24, 2018

I've been confused by the "expanded" qualifier in the past. I wonder if there's a better word to describe it, like "modal"?

I agree the name can be better. This needs to be more generic than "modal" though since it's also used on our dropdowns too. 🤔

@shawnbot
Copy link
Contributor

The dropdowns are modal, though, in the sense that you have to click out of them before you can do anything else — right? Just for argument's sake:

  • "modal" => details-modal
  • "overlay" => details-overlay
  • "layered" => details-layered 🤔

I think details-overlay is my favorite, but I'm not 100% stoked about it. @broccolini, any ideas?

@muan
Copy link
Contributor Author

muan commented May 29, 2018

I think details-overlay sounds good. 👍 Anyone has more ideas?

@shawnbot shawnbot removed the request for review from jonrohan May 29, 2018 22:29
@emplums emplums mentioned this pull request Jun 11, 2018
15 tasks
@jonrohan jonrohan changed the base branch from dev to release-10.6.0 June 13, 2018 22:05
Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

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

Looks good, I'm going to pull this into 10.6.0 #492

@jonrohan jonrohan merged commit ba1b029 into release-10.6.0 Jun 13, 2018
@jonrohan jonrohan deleted the muan/details-el branch June 13, 2018 22:07
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.

4 participants