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

Issue208 Newspaper Template #348

Merged
merged 12 commits into from Oct 19, 2014

Conversation

Projects
None yet
3 participants
@rdaoud1
Contributor

rdaoud1 commented Oct 17, 2014

I have made the newspaper template.

@k88hudson there might be a CSS bug where the first element of the template is hidden behind the top menu navigation bar.

Show outdated Hide outdated lib/templates.json Outdated
@k88hudson

This comment has been minimized.

Show comment
Hide comment
@k88hudson

k88hudson Oct 17, 2014

Member

You're right; there is indeed a CSS bug, although it appears that it's taken care of in #343. Your patch is good otherwise except for some indentation issues in the JSON

In terms of the content of this patch, I think you could do a little more to make this follow the style of a 'Newspaper' article. Check out this screenshot of a Toronto Star article:

image

  • The headline is a larger font size
  • Under the headline is a lede (i.e. a subheading that tells a little more about the article)
  • The photo has a caption and a credit for the photographer
  • The byline (i.e. the author and their affiliation) comes before the article text. Alternatively, it could also go right after the lede. You should consider making a different colour, such as medium grey.

:D

Member

k88hudson commented Oct 17, 2014

You're right; there is indeed a CSS bug, although it appears that it's taken care of in #343. Your patch is good otherwise except for some indentation issues in the JSON

In terms of the content of this patch, I think you could do a little more to make this follow the style of a 'Newspaper' article. Check out this screenshot of a Toronto Star article:

image

  • The headline is a larger font size
  • Under the headline is a lede (i.e. a subheading that tells a little more about the article)
  • The photo has a caption and a credit for the photographer
  • The byline (i.e. the author and their affiliation) comes before the article text. Alternatively, it could also go right after the lede. You should consider making a different colour, such as medium grey.

:D

@rdaoud1

This comment has been minimized.

Show comment
Hide comment
@rdaoud1

rdaoud1 Oct 18, 2014

Contributor

@k88hudson I have revised my newspaper template, any thing else to add to it?

Also, I used an online formatter for the JSON file and I used 4 spaces like you mentioned. I hope that solves my indentation issue.

Contributor

rdaoud1 commented Oct 18, 2014

@k88hudson I have revised my newspaper template, any thing else to add to it?

Also, I used an online formatter for the JSON file and I used 4 spaces like you mentioned. I hope that solves my indentation issue.

@thisandagain

This comment has been minimized.

Show comment
Hide comment
@thisandagain

thisandagain Oct 19, 2014

Member

Hey @rdaoud1! Looking good. One problem is it looks like your last commit caused an issue. It looks like you may have run it through a JSON auto-formatter that caused the build to fail. Can you rollback that last commit (http://stackoverflow.com/questions/927358/undo-the-last-git-commit)? Also, when you write commit messages it's really helpful if you write down exactly what you did that makes it different from the last commit. For example, you have four commits that say something along the lines of "new newspaper template design". It's really helpful if you are explicit about each change.

Thanks and I hope that helps!

Member

thisandagain commented Oct 19, 2014

Hey @rdaoud1! Looking good. One problem is it looks like your last commit caused an issue. It looks like you may have run it through a JSON auto-formatter that caused the build to fail. Can you rollback that last commit (http://stackoverflow.com/questions/927358/undo-the-last-git-commit)? Also, when you write commit messages it's really helpful if you write down exactly what you did that makes it different from the last commit. For example, you have four commits that say something along the lines of "new newspaper template design". It's really helpful if you are explicit about each change.

Thanks and I hope that helps!

@thisandagain

This comment has been minimized.

Show comment
Hide comment
@thisandagain

thisandagain Oct 19, 2014

Member

Yay! Thanks for helping get through that JSON formatting issue!
🍕 🍕 🍕 🍕 🍕

Member

thisandagain commented Oct 19, 2014

Yay! Thanks for helping get through that JSON formatting issue!
🍕 🍕 🍕 🍕 🍕

thisandagain added a commit that referenced this pull request Oct 19, 2014

Merge pull request #348 from rdaoud1/issue208
Issue208 Newspaper Template

@thisandagain thisandagain merged commit ded535e into mozilla:master Oct 19, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment