Rebrand profile page #1352

el-mapache · 2017-04-10T20:46:32Z

Why: Better readability and a mobile first design

Screenshots:

rtwell · 2017-04-10T21:33:39Z

hey @el-mapache this is looking fantastic!

a couple things i spotted that should be addressed before we push this through:

we're missing the Account information header on mobile
the blue in the box-headers should be the same blue as the Welcome… background, #F2F9FF

el-mapache · 2017-04-11T21:02:59Z

@rtwell I changed the blue color, and added back the 'Your account' header on mobile!

el-mapache · 2017-04-11T21:05:19Z

Looking at it now, it seems like we will need a different header entirely for a non-verified account, since the spacing currently takes the presence of the verified badge as a given.

rtwell · 2017-04-11T21:10:16Z

This is how I imagined it:

el-mapache · 2017-04-11T21:16:06Z

@rtwell I just left a comment above re: the spacing, but the email currently only displays on a separate line in the case of a long email — like the homerjsimpson@hotmail.com that you have in the comps.

Should the email always display on a separate line, regardless of length? I'm assuming the name is going to always appear inline unless it is a very long first name.

rtwell · 2017-04-11T21:20:05Z

Ah sorry Adam—i should have specified: Yes, i imagined the email addy, in bold, on its own line, on mobile.

…

On Tue, Apr 11, 2017 at 4:16 PM, Adam B ***@***.***> wrote: @rtwell <https://github.com/rtwell> I just left a comment above re: the spacing, but the email currently only displays on a separate line in the case of a long email — like the ***@***.*** that you have in the comps. Should the email always display on a separate line, regardless of length? I'm assuming the name is going to always appear inline unless it is a very long first name. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1352 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AL-58QyB2WkYXxdpaexoO8QVt09CMUUsks5ru-2ZgaJpZM4M5TP-> .

rtwell · 2017-04-11T21:22:50Z

I realize that is different than the LOA3 instance 😱 but since there is no verified mark, it felt strange sitting by itself. Breaking it onto two lines will deal with the majority of lengthy government email addresses (and avoid truncating them)

el-mapache · 2017-04-11T21:23:57Z

@rtwell understood, will do!

el-mapache · 2017-04-12T13:06:32Z

@rtwell Here is the header for an unverified user:

and a verified one:

hursey013 · 2017-04-13T19:33:20Z

I noticed on LOA1 the "Profile information" heading is still visible, is that expected?

el-mapache · 2017-04-13T19:34:16Z

No, that looks like a merge conflict issue, it wasn't there before 😞

hursey013 · 2017-04-13T19:36:15Z

app/views/profile/_header.html.slim

@@ -1,11 +1,14 @@
-.clearfix.pt2.pb3.sm-py0.bg-light-blue.sm-bg-none
+.clearfix.pt2.pb2.sm-py0.mb2.bg-light-blue.sm-bg-none


Small thing, .pt2.pb2 can be condensed to .py2

el-mapache · 2017-04-13T19:36:56Z

Looking at this again there are now a million problems with the visuals

hursey013 · 2017-04-13T19:47:55Z

app/views/profile/_profile_item.html.slim

@@ -0,0 +1,11 @@
+.p2.clearfix


A few unnecessary grid classes:

.p2.clearfix .col.col-8.clearfix .sm-col.sm-col-6.bold = name .sm-col.sm-col-6 = content if local_assigns.key? :content .col.col-4.right-align - if local_assigns.key? :path = render action, path: path, name: name - else = render action

hursey013 · 2017-04-13T19:59:35Z

app/views/profile/index.html.slim

+    content: current_user.email,
+    path: manage_email_path,
+    action: @view_model.edit_action_partial
+  .border-bottom.border-blue-light


Could this line be added to the containing .p2.clearfix of _profile_item.html.slim instead of having the extra div?

I don't think so, because we want to be able to define when we have a bottom border (e.g. not on the last item)

I went with a border-top to remove this duplication.

hursey013 · 2017-04-13T20:00:44Z

app/views/shared/_nav_auth.html.slim

 nav.bg-white
  .container.px2.sm-py3.pt2.pb0
    .clearfix
-      .sm-col.mb1.sm-m0.col.col-8
+      .sm-col.mb1.sm-m0.col.col-7


.sm-col not necessary here

hursey013 · 2017-04-13T20:01:11Z

app/views/shared/_nav_auth.html.slim

        = link_to \
          image_tag(asset_url('logo.svg'),
            alt: APP_NAME,
            class: 'align-bottom',
            width: 170), root_path
-      .sm-col-right.right-align.col.col-4
+      .sm-col-right.right-align.col.col-5


.sm-col-right not necessary here

hursey013 · 2017-04-13T20:50:21Z

app/assets/stylesheets/components/_background.scss

@@ -0,0 +1 @@
+.bg-gray-lighter { background-color: $gray-lighter; }


We have bg-light-blue and sm-bg-light-blue in util.scss so we might want to consolidate our custom background colors.

Would you prefer to have those classes in util or in a background.scss file?

I'm guessing we may continue to add additional variations so I think having them all in background.scss makes sense.

hursey013 · 2017-04-14T14:05:09Z

vendor/assets/stylesheets/basscss/_border-colors.scss

@@ -11,6 +11,8 @@

 .border-gray   { border-color: $gray }

+.border-blue-lighter { border-color: $blue-lighter; }


I think we should move this into our own border.scss vs the vendor stylesheet just in case we ever update Basscss.

yeah absolutely, I thought I moved this out already!

hursey013 · 2017-04-14T14:29:33Z

This is looking awesome, just noticed a few super small things when running locally then should be good to go.

LOA3 on mobile, Profile information looks like the rounded corners are still present and needs a bottom border:

Bottom border on this gray div ends just a tad bit short:

I think the values in login information and 2fa section need to move one column to the right so all of the values are in line:

hursey013 · 2017-04-14T14:32:27Z

app/views/profile/_pii.html.slim

-.mt3.sm-mt2.mb4
-  .py-12p.border-top
+.border.border-blue-lighter.rounded-md.mb3
+  .bg-light-blue.py2.px2.h6.caps.clearfix


Switch the .py2 to .py1 to match the padding of the other panel headings.

**Why**: CLeaning up view/changing styles to match new designs

**Why**: Trying to keep the partials folder clean

**Why**: Necessary css tweaks, breaks out partial to keep with view model convention

**Why**: Code consistency

**Why**: Other locale files are alphabetized, this should be as well. Use a string to represent the password

**Why**: User shouldn't be able to generate a new personal key without a valid password

**Why**: It wasn't written correctly and would have needed a view context to work properly anyway

**Why**: Its in the comps!

**Why**: Seems like I accidentally revereted some files during a rebase

**Why**: The html stucture needed to change in a few different ways for verified vs non-verified users

**Why**: The code was in a weird state

**Why**: They were the same

**Why**: Codeclimate

**Why**: We need it for build, and we shouldnt have custom styles defined in a vendor stylesheet

**Why**: They were identical to other declarations and therefore not needed

**Why**: Reduces repetition, makes layout look correct

el-mapache · 2017-04-14T15:13:29Z

@hursey013 cleaned up!

hursey013 · 2017-04-14T16:43:20Z

app/assets/stylesheets/components/_border.scss

@@ -10,6 +10,9 @@
 .rounded-xl { border-radius: $border-radius-xl; }
 .rounded-xxl { border-radius: $border-radius-xxl; }

+.border-blue-lighter { border-color: $blue-lighter; }


This doesn't look like it's being used, and after talking it through with @rtwell this can also just fall back to using the default $border-color.

hursey013

this looks good to me, nice work!

**Why**: The profile box tables looked weird. Also moved some files and css declarations around

el-mapache added the status - ready for review label Apr 10, 2017

el-mapache self-assigned this Apr 10, 2017

el-mapache requested a review from hursey013 April 10, 2017 20:46

el-mapache force-pushed the ab-profile-sections branch 3 times, most recently from bcc039b to a324eea Compare April 11, 2017 19:17

el-mapache force-pushed the ab-profile-sections branch 2 times, most recently from 0de4690 to f119a4c Compare April 13, 2017 16:27

hursey013 reviewed Apr 13, 2017

View reviewed changes

el-mapache force-pushed the ab-profile-sections branch 2 times, most recently from 1c72b96 to 6286c0a Compare April 13, 2017 19:52

hursey013 reviewed Apr 13, 2017

View reviewed changes

el-mapache force-pushed the ab-profile-sections branch from c1d2ae2 to 03e58d7 Compare April 13, 2017 20:12

hursey013 reviewed Apr 13, 2017

View reviewed changes

hursey013 reviewed Apr 14, 2017

View reviewed changes

el-mapache added 16 commits April 14, 2017 11:09

Adds new partials, view model methods, styles

b899bce

**Why**: CLeaning up view/changing styles to match new designs

Moves actions into folder, more profile sections

95bb5c0

**Why**: Trying to keep the partials folder clean

CSS tweaks, profile section scss, pkey partial

9c745a2

**Why**: Necessary css tweaks, breaks out partial to keep with view model convention

Address some code climate issue

4c0e249

**Why**: Code consistency

Alphabetize locale files, use string for password

61b881f

**Why**: Other locale files are alphabetized, this should be as well. Use a string to represent the password

Hide p key partial when password_reset present

4aa77df

**Why**: User shouldn't be able to generate a new personal key without a valid password

Remove test method

18c6f67

**Why**: It wasn't written correctly and would have needed a view context to work properly anyway

Adds header text on mobile, light blue in headings

188027e

**Why**: Its in the comps!

Fixes lint errors and merge wierdness

47c2b2b

**Why**: Seems like I accidentally revereted some files during a rebase

Adds partial for non verified header

e76edbe

**Why**: The html stucture needed to change in a few different ways for verified vs non-verified users

More rebasing fixes

9111983

**Why**: The code was in a weird state

Remove duplicate grid classes

2de8477

**Why**: They were the same

Use delegation

9f1fa01

**Why**: Codeclimate

Commits background.scss file

bc4e8bd

**Why**: We need it for build, and we shouldnt have custom styles defined in a vendor stylesheet

Remove extra sm-cols

bf149df

**Why**: They were identical to other declarations and therefore not needed

Use a border-top, fix pii partial nesting

15b10bf

**Why**: Reduces repetition, makes layout look correct

el-mapache force-pushed the ab-profile-sections branch from be8f679 to 028db9c Compare April 14, 2017 15:11

hursey013 reviewed Apr 14, 2017

View reviewed changes

hursey013 approved these changes Apr 14, 2017

View reviewed changes

Offset profile information to match, fixup styles

9490c93

**Why**: The profile box tables looked weird. Also moved some files and css declarations around

el-mapache force-pushed the ab-profile-sections branch from 028db9c to 9490c93 Compare April 14, 2017 16:58

el-mapache merged commit 0b09dd3 into master Apr 14, 2017

el-mapache deleted the ab-profile-sections branch April 14, 2017 17:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rebrand profile page #1352

Rebrand profile page #1352

el-mapache commented Apr 10, 2017

rtwell commented Apr 10, 2017 •

edited

Loading

el-mapache commented Apr 11, 2017

el-mapache commented Apr 11, 2017

rtwell commented Apr 11, 2017

el-mapache commented Apr 11, 2017

rtwell commented Apr 11, 2017 via email

rtwell commented Apr 11, 2017

el-mapache commented Apr 11, 2017

el-mapache commented Apr 12, 2017

hursey013 commented Apr 13, 2017

el-mapache commented Apr 13, 2017

hursey013 Apr 13, 2017

el-mapache commented Apr 13, 2017

hursey013 Apr 13, 2017

hursey013 Apr 13, 2017

el-mapache Apr 13, 2017

el-mapache Apr 13, 2017

hursey013 Apr 13, 2017

hursey013 Apr 13, 2017

hursey013 Apr 13, 2017

el-mapache Apr 13, 2017

hursey013 Apr 14, 2017

hursey013 Apr 14, 2017

el-mapache Apr 14, 2017

hursey013 commented Apr 14, 2017

hursey013 Apr 14, 2017

el-mapache commented Apr 14, 2017

hursey013 Apr 14, 2017

hursey013 left a comment

		@@ -1,11 +1,14 @@
		.clearfix.pt2.pb3.sm-py0.bg-light-blue.sm-bg-none
		.clearfix.pt2.pb2.sm-py0.mb2.bg-light-blue.sm-bg-none

		@@ -0,0 +1 @@
		.bg-gray-lighter { background-color: $gray-lighter; }

		@@ -11,6 +11,8 @@

		.border-gray { border-color: $gray }

		.border-blue-lighter { border-color: $blue-lighter; }

Rebrand profile page #1352

Rebrand profile page #1352

Conversation

el-mapache commented Apr 10, 2017

rtwell commented Apr 10, 2017 • edited Loading

el-mapache commented Apr 11, 2017

el-mapache commented Apr 11, 2017

rtwell commented Apr 11, 2017

el-mapache commented Apr 11, 2017

rtwell commented Apr 11, 2017 via email

rtwell commented Apr 11, 2017

el-mapache commented Apr 11, 2017

el-mapache commented Apr 12, 2017

hursey013 commented Apr 13, 2017

el-mapache commented Apr 13, 2017

Choose a reason for hiding this comment

el-mapache commented Apr 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hursey013 commented Apr 14, 2017

Choose a reason for hiding this comment

el-mapache commented Apr 14, 2017

Choose a reason for hiding this comment

hursey013 left a comment

Choose a reason for hiding this comment

rtwell commented Apr 10, 2017 •

edited

Loading