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.

Sign up for GitHub

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

Jump to bottom

Added page content and styles #7

Merged
merged 1 commit into from
May 1, 2017
Merged

Added page content and styles #7

merged 1 commit into from
May 1, 2017

Conversation

qzhou1607-zz
Copy link
Contributor

  • Homepage
  • Developer Guide
  • User Guide
  • Adjust styles to fit contents generated from markdown
  • Update TOC structure

@aarongustafson
Copy link

Got the PR and pulled it locally. Will look on Monday as it’s time to pick up Oz.

@molant
Copy link
Member

molant commented Apr 28, 2017

Great, thanks a lot!
The html and styles aren't finished yet. We wanted to have an initial commit in the repo and have the initial structure going on so you might see some things off here and there. We will have to decide what we address in this PR and what we open as an issue and fix during the next sprint :)

Have a great weekend!

@molant molant requested a review from ststimac April 28, 2017 23:55
alrra
alrra suggested changes Apr 29, 2017
View reviewed changes
Copy link
Contributor

@alrra alrra left a comment •
edited

Choose a reason for hiding this comment

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

Don't know if all the things in the PR will be used, but will see as we go along, and remove what turns out to not be needed.

themes/documentation/layout/partials/scan-form.hbs Outdated
<div class="home-container--input">
<form action="/scanner" method="POST">
<label for="home-scan">Scan your website for errors...</label>
<input id="home-scan" type="url" placeholder="Copy your URL here" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use: Enter your URL here.

themes/documentation/source/components/accordion/accordion.css
color: #767676;

/* Move up 1/2 height of icon to make it exactly in middle */
-webkit-transform: translateY(-50%);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not important right now, but in the future, when we get to improve the build step, we should use something like autoprefixer.

themes/documentation/source/core/css/code.css Outdated
.highlight {
border: 2px solid hsla(0, 0%, 0%, .03);
background: hsla(0, 0%, 0%, .03);
margin-top: 2.4rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Indent is not consistent.

themes/documentation/source/core/css/code.css Outdated


/*
* GITHUB GIST OVERRIDES
Copy link
Contributor

Choose a reason for hiding this comment

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

We are not (and probably won't be) using GitHub gists, so this section can be removed.

themes/documentation/source/core/css/navigation.css Outdated

margin: 0 auto;

font-family: 'Montserrat', sans-serif;
Copy link
Contributor

Choose a reason for hiding this comment

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

Indent is not consistent.

themes/documentation/source/components/accordion/accordion.css Outdated
/*
* ACCORDIONS
* =============================================
* @Dependencies: base.css, icons.css
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove icons.css as we don't include that file.

themes/documentation/source/components/menu-button/menu-button.css Outdated
/*
* Menu Button
* =============================================
* @Dependencies: base.css, type.css, icons.css
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove icons.css as we don't include that file.

themes/documentation/source/components/pagination/pagination.css Outdated
/*
* Pagination
* =============================================
* @Dependencies: base.css, type.css, icons.css
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove icons.css as we don't include that file.

themes/documentation/source/components/anchor-top/anchor-top.css Outdated

padding: 1.2rem;

font: 2.4rem/1 PlatformIcons;
Copy link
Contributor

Choose a reason for hiding this comment

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

PlatformIcons font is not included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ststimac I don't think we are using platformIcons or any style inside anchor-top.css. Should I just remove the file entirely?

Copy link
Member

Choose a reason for hiding this comment

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

Remove any reference to platform icons.

We may use the anchor-top.css on the results page so leave the rest of it. I haven't finished updating styles/CSS code.

themes/documentation/source/core/css/table-of-contents.css Outdated
*/

.table-of-contents > ul {
margin-top: 1rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Indent is not consistent with the one used in the other files.

@alrra alrra mentioned this pull request May 1, 2017
Closed
@molant
Copy link
Member

molant commented May 1, 2017

@qzhou1607 @ststimac let me know when this is ready for another review.

@ststimac
Copy link
Member

ststimac commented May 1, 2017

@molant my review is done, I'll be providing updated SVGs outside of this PR.

@qzhou1607-zz qzhou1607-zz force-pushed the add-page-content branch 2 times, most recently from 0bf5176 to 008589a Compare May 1, 2017 21:05
@qzhou1607-zz
Copy link
Contributor Author

@molant Ready for another review.

@qzhou1607 @ststimac let me know when this is ready for another review.

@molant
Copy link
Member

molant commented May 1, 2017

Looks @qzhou1607 has addressed most of the feedback and @ststimac will update the svgs via another PR once this is merged.
For the new issues that will appear we should open individual issues so we can address them accordingly.

Merging and closing!

@molant molant merged commit e27a72f into webhintio:master May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alrra alrra alrra requested changes

@aarongustafson aarongustafson Awaiting requested review from aarongustafson

@ststimac ststimac Awaiting requested review from ststimac

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@qzhou1607-zz @aarongustafson @molant @ststimac @alrra