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

feat: add disclosure button (resolves #150) #154

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

greatislander
Copy link
Collaborator

Description

Adds disclosure button component.

Steps to test

Review component:

Additional information

Not applicable.

Related issues

@greatislander greatislander added the enhancement New feature or request label Jan 16, 2020
@greatislander greatislander added this to the 1.0.0-alpha.6 milestone Jan 16, 2020
@greatislander greatislander self-assigned this Jan 16, 2020
@jhung
Copy link
Contributor

jhung commented Jan 16, 2020

I was reviewing this component and I realized that depending on the context, it is semantically better to use the native <summary><details> pattern instead of a button with Javascript. Summary-details would make sense for some of the text content such as notifications and paragraph text.

See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details

Summary-Details should not be used in cases like the sidebar navigation - semantically it won't make sense as a parent item is not a summary of the sub items.

All that said, this disclosure component is still useful, but summary-detailis should be used instead where it is appropriate.

@greatislander greatislander merged commit 76b22c4 into dev Jan 16, 2020
@greatislander greatislander deleted the add/disclosure-button branch January 16, 2020 16:07
@greatislander
Copy link
Collaborator Author

@jhung I've merged as-is. It looks like there's some weird workarounds needed to style summary/details the way the designs specify, and it would need to be polyfilled for IE/non-Chromium Edge. Can you create a follow-up issue for this?

@cherylhjli
Copy link

Is there a way to bump the up chevron arrow down a little bit, or would that affect the down chevron as well?

@greatislander
Copy link
Collaborator Author

It would affect the down chevron as well and if the vertical positioning changed between states the rotation would look weird (it would shift up/down while rotating).

@cherylhjli
Copy link

ok, don't worry about it then! it works as it is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Atom: disclosure button
3 participants