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.

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

Landing page HTML and CSS #244

Merged
merged 30 commits into from
Jun 12, 2017
Merged

Landing page HTML and CSS #244

merged 30 commits into from
Jun 12, 2017

Conversation

Cleop
Copy link
Member

@Cleop Cleop commented Jun 8, 2017

#241

The burger for the mobile menu and that functionality is being addressed in a separate issue #249

@Cleop Cleop added the in-progress An issue or pull request that is being worked on by the assigned person label Jun 8, 2017
@Cleop Cleop self-assigned this Jun 8, 2017
@Cleop Cleop added the awaiting-review An issue or pull request that needs to be reviewed label Jun 12, 2017
@Cleop Cleop changed the title WIP - Landing page HTML and CSS Landing page HTML and CSS Jun 12, 2017
@Cleop Cleop requested review from nelsonic and SimonLab June 12, 2017 17:03
@Cleop Cleop assigned nelsonic and SimonLab and unassigned Cleop Jun 12, 2017
@Cleop Cleop removed the in-progress An issue or pull request that is being worked on by the assigned person label Jun 12, 2017
@nelsonic nelsonic added in-review Issue or pull request that is currently being reviewed by the assigned person and removed awaiting-review An issue or pull request that needs to be reviewed labels Jun 12, 2017
Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

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

@Cleop looks great! 🎉

@nelsonic nelsonic added awaiting-review An issue or pull request that needs to be reviewed and removed in-review Issue or pull request that is currently being reviewed by the assigned person labels Jun 12, 2017
@nelsonic nelsonic assigned iteles and unassigned nelsonic Jun 12, 2017
<textarea id="message" name="message" class="br2 bw1 w-90 dwyl-b--dark-grey pa2" rows="10" placeholder="What problem can we solve for you?"></textarea>
</fieldset>

<button class="bn dib link mv2 ml3 ph3 pv3 dwyl-bg-mint white br2 f4 shadow-4">
Copy link
Member

Choose a reason for hiding this comment

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

Should we add the pointer class on the button?

Copy link
Member

Choose a reason for hiding this comment

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

Should we centre this button on mobile?

<h1 class="mt0-ns mb3 pl4-ns f-xl tl fw9">Let's</h1>
<h1 class="pl4-ns mt3 f-xl tl fw9">Talk</h1>
</div>
<div class="w-80-ns w-90 ml3">
Copy link
Member

Choose a reason for hiding this comment

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

We should add some links with mailto, tel and google maps

Copy link
Member

Choose a reason for hiding this comment

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

</div>
</ul>
</div>
<a href="#" class="dib link mt2 mb4 ml4 ml3-ns ml6-ns ph4 pv2 dwyl-bg-mint white br2 f5 shadow-4">Enquire Now</a>
Copy link
Member

Choose a reason for hiding this comment

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

Should we centre this link on mobile?

Copy link
Member

@SimonLab SimonLab left a comment

Choose a reason for hiding this comment

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

Looks good 👌 . I've added a few comments which can be addressed on the next issues. @iteles do you have any other feedback?

@SimonLab SimonLab removed their assignment Jun 12, 2017
@iteles iteles added in-review Issue or pull request that is currently being reviewed by the assigned person and removed awaiting-review An issue or pull request that needs to be reviewed labels Jun 12, 2017
Copy link
Member

@iteles iteles left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @Cleop!

Please take a look at mine and @SimonLab's comments, though these aren't blocking a merge and can be fixed in subsequent PRs.

Also, definitely worth splitting something like this into smaller chunks so the PRs are a little easier to digest 😊

<title>dwyl</title>
</head>
<body>
<h1>Hello world!</h1>
<nav class="w-100 vh-100 h3-ns dwyl-bg-dark-grey tc white">
Copy link
Member

Choose a reason for hiding this comment

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

Only partial support in IE for vh and I know this caused @RobStallion some issues on FREED - please test this in IE10 + 11 (@Danwhy has a good way of doing this with saucelabs manual testing and https://ngrok.com/ )

Copy link
Member Author

Choose a reason for hiding this comment

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

This has been removed as a rem unit is now suitable with the new design.

<meta name="keywords" content="do, what, you, love, dwyl, mvp, time, tracking, help, developers, code, frontend, backend, tooling, tools">
<meta name="robots" content="index, follow, noarchive">
<meta name="base" content="https://www.dwyl.com">
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/font-awesome/4.4.0/css/font-awesome.min.css">
Copy link
Member

Choose a reason for hiding this comment

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

We might want to consider a customised version of fontawesome at a later stage when we have a back end if we feel there are performance benefits from it.

<h1 class="mt0-ns mb3 pl4-ns f-xl tl fw9">Let's</h1>
<h1 class="pl4-ns mt3 f-xl tl fw9">Talk</h1>
</div>
<div class="w-80-ns w-90 ml3">
Copy link
Member

Choose a reason for hiding this comment

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

@iteles iteles merged commit d238a0c into master Jun 12, 2017
@iteles iteles deleted the landing-page branch June 12, 2017 20:34
@Cleop
Copy link
Member Author

Cleop commented Jun 13, 2017

Thanks for the feedback 👍 I've spoken to @harrygfox to get his feedback. Now I'll create new issues for amends to be made. Following this, the Values page will be next.

@Cleop Cleop mentioned this pull request Jun 13, 2017
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-review Issue or pull request that is currently being reviewed by the assigned person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants