Skip to content
This repository has been archived by the owner on Jan 15, 2020. It is now read-only.

Adding Getting Started page #29

Merged
merged 1 commit into from Aug 11, 2018
Merged

Adding Getting Started page #29

merged 1 commit into from Aug 11, 2018

Conversation

dhmlau
Copy link
Member

@dhmlau dhmlau commented Jul 27, 2018

Fixes #22

@raymondfeng
Copy link
Member

Should it be merged into gh-pages or master?

@dhmlau
Copy link
Member Author

dhmlau commented Aug 7, 2018

we're using the master branch

Copy link
Contributor

@virkt25 virkt25 left a comment

Choose a reason for hiding this comment

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

Awesome effort!! Just a few things seem off from the spec but should be a quick fix 😇

color: black;
background-color: var(--color-white-xd);
padding: 5px;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a newline here. I think we need to set up linting for this repository.

Copy link
Member Author

Choose a reason for hiding this comment

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

let me try to set up linting or get some help from you on setting this up. thanks.

css/loopback.css Outdated
width: 100%;
position: relative;
top: 40%;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Newline.

</div>
<div class="col-md-6">
<h3 class="feature-heading">Have you installed Node.js?</h3>
<p class="p large">Before you install LoopBack, make sure to download and install Node.js (version 8.x.x or higher), a JavaScript
Copy link
Contributor

Choose a reason for hiding this comment

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

This should say version 8.9.x or higher ... and maybe we should abbreviate version to v? v 8.9.x or higher?

runtime. </p>
</div>
<div class="col-md-6">
<a class="btn btn-secondary" style="padding-left:60px; padding-right:60px" role="button" href="https://nodejs.org/en/download/">Install Node.js</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't need inline styling for this button! Also, please check the hover state of this button, something seems wrong with the border.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

for some reason, the border color is changed when it's in hover. will fix that. thx

</p>
</div>
<div class="col-md-6">
<p class="code-snippet" style="top:20%">
Copy link
Contributor

Choose a reason for hiding this comment

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

Code snippet should go in a <pre> block so space is preserved. Any reason to not use that here?

Copy link
Member Author

Choose a reason for hiding this comment

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

will fix this and other occurrences. thx

</p>
</div>
<div class="col-md-6">
<p class="code-snippet">
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a <pre> block to preserve spacing ... and spacing / alignment should be fixed, for example on the last line the brackets shouldn't be there but rather moved to their own line and the return statement should be indented two levels in while the function is indented one level in.

<div class="container">
<div class="row text-center">
<div class="col-md-6">
<p class="p large" style="position:relative; top:50%">Answer the prompts as follows.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't aligned with the code block on the right. Same goes for the other sections.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure if i know what you mean. you mean it's not aligned vertically?

Copy link
Member Author

Choose a reason for hiding this comment

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

i've made it now so that the left text is somewhat in the middle relatively to the right column.

<div style="height:80px"></div>
<h2>Ready to build with LoopBack?</h2>
<br>
<h5 style="font-weight: normal">Let's get you eqipped</h5>
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a . at the end of the sentence.

Copy link
Contributor

@shimks shimks Aug 8, 2018

Choose a reason for hiding this comment

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

tagging along here:
eqipped?
did you mean equipped?

Copy link
Contributor

Choose a reason for hiding this comment

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

OMG I knew something about the word looked off but couldn't figure out what.

</div>
</nav>
<!-- Header -->
<div class="gs-masthead">
Copy link
Contributor

@virkt25 virkt25 Aug 8, 2018

Choose a reason for hiding this comment

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

This background image needs to be muted -- spec seems to have an opacity overlay from top to bottom which gives better readability of the text on this (especially the smaller second line). Please implement that!

image

vs.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll change the opacity from 0.4 to 0.2 in the new version. Hope it's better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well opacity applies to the entire image right? I think you need a gradient ... as it's lighter in the top and less so in the bottom.

Copy link
Member Author

Choose a reason for hiding this comment

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

got it

Copy link
Contributor

Choose a reason for hiding this comment

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

It's looks amazing with the gradient! 🔥

<!-- Header -->
<div class="gs-masthead">
<div class="container-fluid"></div>
<div style="height:80px"></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Inline styles should be avoided as much as possible.

@dhmlau
Copy link
Member Author

dhmlau commented Aug 9, 2018

@virkt25 @shimks , I've made a few more changes according to your feedback (@shimks gave his verbally):

  • spacing below the step title
  • hairline under the navbar
  • gradient opacity on the main image of index and getting started page
  • code snippet formatting

Copy link
Member

@b-admike b-admike left a comment

Choose a reason for hiding this comment

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

Can we align the text for the following?
screen shot 2018-08-09 at 3 25 39 pm

Other than that, LGTM

@dhmlau
Copy link
Member Author

dhmlau commented Aug 9, 2018

Thx @b-admike. Fixed it.

Copy link
Contributor

@virkt25 virkt25 left a comment

Choose a reason for hiding this comment

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

Great job!! Almost there. Some minor nitpicks.

<p class="p large">Follow the tutorial to create a real-world application with LoopBack 4.</h3>
</div>
<div class="col-md-4">
<a class="btn btn-primary" role="button" style="padding-left:60px; padding-right:60px" href=https://loopback.io/doc/en/lb4/todo-tutorial.html>View Tutorial</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you confirm the hover effect for this button? Right now it just changes the font color.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

confirmed with @shimks , only font color is changed. I will submit another PR for the hover spec that I got from Cody after his initial file.

</div>
<div class="col-md-6">
<h3 class="feature-heading">Test your application</h3>
<p class="p large step">Start the application using <code>npm start</code>.
Copy link
Contributor

Choose a reason for hiding this comment

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

The background for <code> blocks here and elsewhere looks more darker in the spec.

Copy link
Member Author

Choose a reason for hiding this comment

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

screen shot 2018-08-10 at 5 05 18 pm

hm. should be the same. it's "White-XD"

</p>
</div>
<div class="col-md-6">
Visit <a class="link" href="http://127.0.0.1:3000/hello">http://127.0.0.1:3000/hello</a> to see <code>Hello world!</code>
Copy link
Contributor

Choose a reason for hiding this comment

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

Confirm hover effect for links. Right now it just becomes a dark blue + underline (this is the default browser style I think) ... looks out of place imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

screen shot 2018-08-10 at 5 09 19 pm

it should be dark blue but no underline. will remove the underline text decoration.

<hr class="hairline">
<div class="row text-center">
<div class="col-md-6">
<p class="p large" style="position:relative;top:40%">Paste the following contents into the file <code>/src/controllers/hello.controller.ts</code>.
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 aligned to be in the middle of the code block on the right.

<code-blue> $</code-blue> lb4 controller
<code-blue>[?]</code-blue> Controller class name: hello
<code-blue>[?]</code-blue> What kind of controller would you like to generate? Empty Controller
create src/controllers/hello.controller.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Latest version of the CLI should also be updating the index.ts file and that should be shown here.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated with the new output.

</div>
<div class="col-md-6">
<h3 class="feature-heading">Install LoopBack 4 CLI</h3>
<p class="p large step">The LoopBack 4 CLI is a command-line interface that can scaffold a project or extension. The CLI provides
Copy link
Contributor

Choose a reason for hiding this comment

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

Good in terms of spec but we should see if we can reduce the amount of text here, it's a bit overwhelming.

Copy link
Member Author

Choose a reason for hiding this comment

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

would like to do it in another PR.

runtime. </p>
</div>
<div class="col-md-6">
<a class="btn btn-secondary" style="padding-left:60px; padding-right:60px; top: 10%" role="button" href="https://nodejs.org/en/download/">Install Node.js</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

This button doesn't seem perfectly in the middle of the text to the right.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

</div>
</nav>
<!-- Header -->
<div class="gs-masthead">
Copy link
Contributor

Choose a reason for hiding this comment

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

It's looks amazing with the gradient! 🔥

Signed-off-by: Diana Lau <dhmlau@ca.ibm.com>
@dhmlau dhmlau merged commit aaee52e into master Aug 11, 2018
@dhmlau dhmlau deleted the gs branch August 11, 2018 13:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants