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

PA Review: User Auth #2

Open
6 tasks done
jomartinez27 opened this issue Aug 7, 2018 · 5 comments
Open
6 tasks done

PA Review: User Auth #2

jomartinez27 opened this issue Aug 7, 2018 · 5 comments

Comments

@jomartinez27
Copy link
Owner

jomartinez27 commented Aug 7, 2018

  • Backend: DB (UsersController, SessionController, User Model)
  • Add seeds
  • Redux Cycle: (ajax, actions, reducer, container)
  • Presentational Component (signup, login, logout nav)
  • Styling
  • Smooth navigation
@s-pangburn
Copy link
Collaborator

s-pangburn commented Aug 13, 2018

Looking good Jose! Great work!

  • Your errors actually push the demo user button and everything else off the form, take a look:
    screen shot 2018-08-13 at 10 51 29 am
    • You could probably get away with a lot less spacing on the top there, especially between errors, so try to clean this up a little bit and make it so things don't jump down quite so much.
  • Upon logging in, your dropdown avatar icon should cursor: pointer on hover.
    • Same with the "Logout" <li>
  • Your navbar size is inconsistent between the login and signup pages, and the placement of "PixelPx" jumps all over the place (even at root). Try your best to fix this, because having it in a fixed spot and size is more professional.
  • The gray background on your login/signup pages doesn't reach all the way to the bottom.
  • Your inputs should have just a little more padding on the left and right. It's a little hard to read right now when it's too close to the edge.
    • Also, I wouldn't recommend using a serif font (like Times New Roman) for your buttons and inputs. Modern web design language prefers rounded corners, so a rounded sans-serif fonts will look more "professional" to people (even though it's really just a matter of personal taste)
  • There should be some indicator of my username upon logging in for now (at least until you add avatars or something) so I know I'm in the right account (some people have a bug where they only ever log in as a guest)
  • You seem to allow duplicate emails?
  • Going to a random route https://picturepixel.herokuapp.com/#/sldkfjsldkfjsldf should redirect to root.

Try your best to have these things addressed asap and I'll take another look!

@jomartinez27
Copy link
Owner Author

Alrighty Stephen! looks like I managed to get these all done. On the front page I have a github picture link I found from here:
https://github.com/moshen/Image-Term256Color

I see a copyright claim on it and don't know if I can use it.

@jomartinez27
Copy link
Owner Author

WAIT changing the font made my session form break again!

@jomartinez27
Copy link
Owner Author

Alrighty all set

@s-pangburn
Copy link
Collaborator

s-pangburn commented Aug 13, 2018

Alright, looking good! Just a couple things left:

  • When I open the console, your splash image shrinks and exposes the white at the bottom of the page. Try your best to fix this:
    screen shot 2018-08-13 at 3 15 01 pm

  • The github pixel octocat, while cute, lacks a transparent background, so it looks a little out of place and it's not immediately clear it's a link to your GitHub. I'd recommend a simple icon in the navbar, but if you're going to keep it at the bottom, 1) Choose something super visible and 2) add some padding on the left!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants