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

Add HTML example for <details> #924

Merged
merged 2 commits into from May 18, 2018

Conversation

Projects
3 participants
@Regaddi
Contributor

Regaddi commented May 15, 2018

No github issue created so far.
I added some more styles to the element to make clear that it is a fully customizable element.

@welcome

This comment has been minimized.

welcome bot commented May 15, 2018

💖 Thanks for opening this pull request! 💖
Here is a list of things that will help get it across the finish line: - If this is a new or updated CSS interactive example, please ensure that you followed the CSS styleguide - If this is a new or updated JavaScript interactive example, please ensure that you followed the JavaScript styleguide - If your changes affects any of the steps in our contribution docs, please also make the relevant changes there.

@schalkneethling schalkneethling self-requested a review May 17, 2018

@schalkneethling schalkneethling self-assigned this May 17, 2018

@schalkneethling schalkneethling added this to To do in Examples Needed via automation May 17, 2018

@schalkneethling schalkneethling added this to To do in Q2 Sprint 3 via automation May 17, 2018

@@ -0,0 +1,41 @@
details {
border: 1px solid #aaaaaa;

This comment has been minimized.

@wbamberg

wbamberg May 17, 2018

Member

We use short form hex codes when we can, so this should be #aaa.

font-weight: bold;
margin-left: -.5em;
margin-right: -.5em;
margin-top: -.5em;

This comment has been minimized.

@wbamberg

wbamberg May 17, 2018

Member

Perhaps use a single margin declaration here?

summary::-webkit-details-marker {
display: none;
}

This comment has been minimized.

@wbamberg

wbamberg May 17, 2018

Member

This (obviously) doesn't work in Firefox and ends up looking weird:

screen shot 2018-05-17 at 11 49 05 am

I like what you have here, but think it would be better to omit the "+"/"-" markers.

@wbamberg

Thanks for your contribution @Regaddi ! I had a few quite minor comments.

Also though - we don't unfortunately note this in the style guide and haven't been terribly consistent with asking for it, but I prefer examples which are not themselves documentation of the thing for which they are examples. I think this is potentially confusing for people: for example, in our user testing some people saw the contents of the output pane as part of the documentation itself, and I think choosing documentation as the example will encourage that.

So I'd prefer examples that are clearly something else, like https://interactive-examples.mdn.mozilla.net/pages/tabbed/caption.html or https://interactive-examples.mdn.mozilla.net/pages/tabbed/footer.html.

details[open] summary {
border-bottom: 1px solid #aaa;
color: #aaa;

This comment has been minimized.

@wbamberg

wbamberg May 17, 2018

Member

I think changing font color (at least changing it this much) is not needed and hurts contrast too much. But this is debatable I suppose :).

@Regaddi

This comment has been minimized.

Contributor

Regaddi commented May 17, 2018

@wbamberg Thank you very much for the detailed review, I really appreciate your input!

I adjusted styles and content accordingly. Using the ::-webkit-details-marker was truly insufficient. I removed it to engage simplicity in the example.

@wbamberg

This looks great to me, thanks for your contribution @Regaddi :)

@wbamberg wbamberg merged commit 283ff97 into mdn:master May 18, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json No dependency changes
Details

Examples Needed automation moved this from To do to Done May 18, 2018

Q2 Sprint 3 automation moved this from To do to Done May 18, 2018

@welcome

This comment has been minimized.

welcome bot commented May 18, 2018

Congrats on merging your first pull request! 🎉🎉🎉

wbamberg added a commit to wbamberg/interactive-examples that referenced this pull request May 25, 2018

Merge remote-tracking branch 'upstream/master'
* upstream/master: (39 commits)
  Add video example. (mdn#939)
  Add track example. (mdn#940)
  Change `html` to `tabbed` in Publishing section (mdn#942)
  adding font-optical-sizing example (mdn#919)
  chore(deps): update dependency jest to v22.4.4 (mdn#935)
  fix(tabbed-editor): issue mdn#860, isolate CSS to output (mdn#927)
  Add img example. (mdn#923)
  Add <area> example (mdn#920)
  Add <map> example (mdn#931)
  Add HTML example for <audio> (mdn#930)
  chore(community): add @Regaddi as contributor (mdn#933)
  chore(community): add @stephanmax as contributor (mdn#932)
  Add HTML example for <summary> (mdn#926)
  Add HTML example for <details> (mdn#924)
  Fix example: issue mdn#925, add (max-)width to container and left-align text to see full effect (mdn#929)
  Add String trim examples (mdn#922)
  chore(deps): update dependency stylelint to v9.2.1 (mdn#928)
  Add example for week input (mdn#902)
  Add url input example (mdn#901)
  Add html input search (mdn#897)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment