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

Align web app UI to website design #679

Merged
merged 2 commits into from
Jun 8, 2016
Merged

Conversation

ryankeairns
Copy link
Contributor

@ryankeairns ryankeairns commented Jun 8, 2016

The big additions/changes are the nav/global navigation and footer to match the new website design (including smaller device size support... although there will be more work to refine this, it is functional).
There are many additional smaller tweaks related to global layout and branding.

screenshot 2016-06-08 12 30 21
screenshot 2016-06-08 12 30 35

Ryan Keairns added 2 commits June 7, 2016 15:23
@thesentinels
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @smith and @fnichol to be potential reviewers

</footer>
</div>`,
</div>
<footer class="main-footer">
Copy link
Contributor

Choose a reason for hiding this comment

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

The footer is big enough now that it should be broken into its own component.

@smith
Copy link
Contributor

smith commented Jun 8, 2016

This is beautiful.

A few things need fixing:

  • The sign up link show up whether or not you've signed in
  • I thought the link said "sign in" before but now it's "sign up." I think "sign in" is clearer because there's not really a sign up process
  • When you're signed in, the dropdown for the user doesn't work correctly
  • When you're signed in, the GitHub avatar used to have a border around it, but now it's just the square icon
  • If you're on the sign in page while signed in, "signed in with github" and "sign out" buttons have different border radii
  • Make footer own component
  • Change href on sign in links in top nav
  • Change footer links to use config vars
  • Fix inconsistent spaces in HeaderComponent.ts
  • Consolidate new javascript files into build process
  • Clean up CSS for active nav background

</ul>
<div class="main-nav--container clearfix">
<div class="main-nav--logo">
<a href="/"><h1>{{appName}}</h1></a>
Copy link
Contributor

Choose a reason for hiding this comment

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

There's inconsistent use of 2/4 spaces in here.

@smith
Copy link
Contributor

smith commented Jun 8, 2016

@thesentinels r+

@thesentinels
Copy link
Contributor

📌 Commit 8edc7d6 has been approved by smith

@thesentinels
Copy link
Contributor

⌛ Testing commit 8edc7d6 with merge a902e31...

thesentinels pushed a commit that referenced this pull request Jun 8, 2016
Signed-off-by: Ryan Keairns <[email protected]>

Pull request: #679
Approved by: smith
thesentinels pushed a commit that referenced this pull request Jun 8, 2016
Signed-off-by: Ryan Keairns <[email protected]>

Pull request: #679
Approved by: smith
@thesentinels
Copy link
Contributor

☀️ Test successful - travis

@thesentinels thesentinels merged commit 8edc7d6 into master Jun 8, 2016
jtimberman pushed a commit that referenced this pull request Jun 12, 2016
Signed-off-by: Ryan Keairns <[email protected]>

Pull request: #679
Approved by: smith
jtimberman pushed a commit that referenced this pull request Jun 12, 2016
Signed-off-by: Ryan Keairns <[email protected]>

Pull request: #679
Approved by: smith
@smurawski smurawski deleted the rk/webapp-treatyoself branch February 3, 2017 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants