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

Implemented example for <figcaption> tag #941

Merged
merged 5 commits into from May 28, 2018

Conversation

3 participants
@dagolinuxoid
Contributor

dagolinuxoid commented May 19, 2018

Take a look. I used few lines of HTML and 22 lines of CSS — a little bit of scrolling won't hurt.

@welcome

This comment has been minimized.

welcome bot commented May 19, 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.

@dagolinuxoid dagolinuxoid changed the title from Implemented example for <figcapture> tag to Implemented example for <figcaption> tag May 19, 2018

@wbamberg wbamberg self-requested a review May 24, 2018

@wbamberg

Thanks for your contribution, and sorry to be so slow reviewing this. In general this looks great!

I just have a couple of very nitpicky comments.

One other comment is that we're always attributing the images we use. In this case we'd use something like:

Photo by Ricky Kharawala on Unsplash

Perhaps we could include it in the figcaption itself, by making it something like:

Hamster by Ricky Kharawala on Unsplash

...if you do that, it would probably be worth setting margin: 1rem; or something like that on the figure, so the caption gets a bit more horizontal space and won't wrap so readily.

background: #222;
color: #fff;
padding: 3px 0;
}

This comment has been minimized.

@wbamberg

wbamberg May 26, 2018

Member

Files should end with a newline, here and in the other cases.

@@ -0,0 +1,4 @@
<figure>
<img src="/media/examples/hamster.jpg" alt="hamster">

This comment has been minimized.

@wbamberg

wbamberg May 26, 2018

Member

In these examples we prefer to close tags even when it's not needed:

<img src="/media/examples/hamster.jpg" alt="hamster" />

@dagolinuxoid

This comment has been minimized.

Contributor

dagolinuxoid commented May 26, 2018

@wbamberg, thanks for a thorough review. I made some fixes so now it looks decent... I guess :)

@wbamberg

Looks good to me @dagolinuxoid ! Thanks for your contribution!

@wbamberg wbamberg merged commit c22cf71 into mdn:master May 28, 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 28, 2018

@welcome

This comment has been minimized.

welcome bot commented May 28, 2018

Congrats on merging your first pull request! 🎉🎉🎉

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

Merge remote-tracking branch 'upstream/master'
* upstream/master:
  adding oblique plus angle option to font-style example (mdn#963)
  fix(tabbed-editor): Apply output class to output container (mdn#961)
  Add rt example. (mdn#957)
  chore(deps): update dependency prettier to v1.13.4 (mdn#953)
  chore(deps): update dependency all-contributors-cli to v4.11.2 (mdn#954)
  chore(deps): update dependency jest to v23.1.0 (mdn#955)
  Fix console util to support negative zero (mdn#960)
  Add rp example. (mdn#945)
  Bug 1462897 - Directly use Math.round in the demo code. (mdn#956)
  chore(docs): update README with maintainers and good first issues (mdn#934)
  Implemented example for <figcaption> tag (mdn#941)
  Font variation settings (mdn#948)
  Fix font optical sizing (mdn#947)
  chore(deps): update dependency jest to v23 (mdn#944)
  chore(deps): update dependency prettier to v1.13.0 (mdn#950)
  chore(community): add @elharony as contributor (mdn#951)
  Add object-keys.html (mdn#937)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment