1020981 - Remove Edit Profile button from signed in Front Page view

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: One and Done, defect, P3)

Description

[Rebecca Billings [:rbillings]]

Remove the Edit Profile button from the Front Page signed in view. There is already a link to the profile in the header, and this button takes them away from tasks.

Comment 1

[Bob Silverberg [:bsilverberg]]

I'll do this. I will only take a few minutes.

Comment 2

[Bob Silverberg [:bsilverberg]]

Attachment: Link to Github pull-request: https://github.com/mozilla/oneanddone/pull/125

Bitgeeky or Maja, can you please review this patch by testing it locally, and also reviewing the code?

Comment 3

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8434987 [details] [review]
Link to Github pull-request: https://github.com/mozilla/oneanddone/pull/125

Didn't find any issues, Bob, looks good. Small suggestion about CSS -- see PR.

Comment 4

[Pankaj Malhotra (:bitgeeky)]

Code is fine Bob but TBH I really don't feel its the right place for edit profile button where it is currently placed.
In fact IMO the design for profile page needs to be given a re-thought. 
The way profile name is displayed, placement of edit profile button and having two separate content-containers on the same page really don't look good.

Rebecca can you please look into this ?

Comment 5

[Rebecca Billings [:rbillings]]

The edit profile bug 1018321 will update some of the display. I'm unclear what you mean about placement of the button- so please bring it up during the dev discussion meeting or file a bug with an alternative suggestion as this bug is only about removing the Edit Profile button from the front page.

Comment 6

[Pankaj Malhotra (:bitgeeky)]

If we remove the edit profile button from front page we also have to keep it in view profile button as to navigate to edit profile page.
So I am talking about its placement here.
Where should the edit profile button be placed in view profile page ?

Comment 7

[Rebecca Billings [:rbillings]]

The edit profile button will be on the view profile page, as described in bug 1005114

Comment 8

[Pankaj Malhotra (:bitgeeky)]

Attachment: edit_profile_button.png

Thanks Rebecca that makes it more clear.

The attachment is what I see on the latest commit with the pull request Bob made rather than how it looks here https://bug1005114.bugzilla.mozilla.org/attachment.cgi?id=8421699

Also the pull request from Maja https://github.com/mozilla/oneanddone/pull/124 which got r+ is not in accordance with this https://bug1005114.bugzilla.mozilla.org/attachment.cgi?id=8421699

Bob can you please tell why it was so or am I missing something ?

Comment 9

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

My understanding of working with this kind of mock-up is that it's approximate, although perhaps that is not the case for this project. Regarding styling, I think Bob closed bug 1005114 because people seemed content with how things look during our last check-in meeting. I generally wouldn't fiddle with styling anyway until the functionality is satisfactory and we get specific feedback about how things look. 

(In reply to Pankaj Malhotra (:bitgeeky) from comment #8)
> Also the pull request from Maja
> https://github.com/mozilla/oneanddone/pull/124 which got r+ is not in
> accordance with this
> https://bug1005114.bugzilla.mozilla.org/attachment.cgi?id=8421699
> 
> Bob can you please tell why it was so or am I missing something ?

Comment 10

[Pankaj Malhotra (:bitgeeky)]

I understand Maja that functionality is fine but we also need to care about design issues.

The meetings we have are very short and we have lots of discussion items so everybody can't give an in-depth review but Wireframe/Design Reference goes though lots of reviews before being finalized and we shall stick with it or raise a design issue if it doesn't appeal to you.

So in this case also I would recommend sticking with the design that was proposed in Wireframe/ Design Reference.

Comment 11

[Pankaj Malhotra (:bitgeeky)]

I will recommend to re-open https://bugzilla.mozilla.org/show_bug.cgi?id=1005114  and address the design issues.

Comment 12

[Bob Silverberg [:bsilverberg]]

I share Maja's point of view that we need to separate functional changes from design issues. If we start fiddling with design during PR reviews we'll never get anything accomplished. Design is very easy to address as long as all of the elements are there and they work. So it can be done later in the game.

To be honest I didn't even look at the wireframe for the Edit Profile page when making the change as the bug was about removing it from the home screen. I removed it, but then realized that it needed to be somewhere, so I added it to the profile detail.

I really should have looked at the wireframe and attempted to come up with something close, so I will do that, but it's not out of the question that I will come up with something we don't like from a design perspective and that can be addressed in a separate bug.

As far as I know we don't have a UX resource, so I'm not sure who, if anyone, will be reviewing the design of what we do. I think if things deviate significantly from the wireframe, from a functionality perspective (e.g., missing fields or controls) then that should be a blocker, but if things just don't quite look like the wireframes then I think we can proceed with reviews and merges. Rebecca and Liz can view the results on the staging site and then raise bugs if the design needs to change.

Comment 13

[Bob Silverberg [:bsilverberg]]

I moved the button to the bottom right, with the proper styling. :bitgeeky, one of the reasons that the screen doesn't look like the wireframe is because it includes the dashboard, which is different from the mockup. I expressed some concerns about this in our checkin meeting, but everyone was totally fine with the way it looks now, so there's no reason to change it to match the wireframe when it got everyone's sign off. 

As stated above (and sorry if I sound like a broken record), we really need to just focus on functionality changes for now to meet our deadlines, and let Rebecca, Liz and the community comment on whether it looks the way it needs to when they see the results on staging.

Comment 14

[Pankaj Malhotra (:bitgeeky)]

Attachment: user_profile.png

Comment 15

[Pankaj Malhotra (:bitgeeky)]

I am sorry but I still hold my view and not convinced with the points you make here because I have all the reasons to do so.

1 Why can't we be consistent with what was proposed in Wireframe/Design Reference when the feature request was made ?

2 Why do you want to open two separate bugs(one for functionality and one for design) for a single feature request ?

3 Why can't we spend a few more minutes to address the design issues rather than make a pull request with just the functionality working ?

If it has been just about the fonts and how text is displayed it would have been a very minor issue but this one is about placement of elements on the page.

The elements in this page aren't where they need to be and the overall design is not consistent with rest of the pages of application.

I will leave it to Rebecca and Liz to review the design and :Osmose to comment on its consistency with rest of application pages.

This https://bug1020981.bugzilla.mozilla.org/attachment.cgi?id=8435535 is the current version and this https://bug1005114.bugzilla.mozilla.org/attachment.cgi?id=8421699 is what was proposed.

Please keep the following points in mind while giving your feedback:
1 The placement of user profile name.
2 The placement of pick a task button and edit profile button.
3 Two separate content-containers which seem to divide the page in two parts.

Comment 16

[Bob Silverberg [:bsilverberg]]

(In reply to Pankaj Malhotra (:bitgeeky) from comment #15)

:bitgeeky, your points are valid, but you seem to be ignoring one thing that both Maja and I have mentioned above: The current design (which I too don't particularly like, and that's why I brought it up for discussion in Wednesday's meeting) has been reviewed and approved by not only Rebecca and Liz, but also the wider QA community in Wednesday's meeting. So whether or not you or I don't particularly like it, it seems somewhat counterproductive to question it at this point when it's already been reviewed and approved. We have so many other tasks to do in a short period of time - we don't really need to go looking for extra work when something has already been signed off.

My point is that we should concentrate on getting those tasks done, and revisit design at the end if we feel it needs re-addressing.

I will also try to provide a response to your individual points:

> I am sorry but I still hold my view and not convinced with the points you
> make here because I have all the reasons to do so.
> 
> 1 Why can't we be consistent with what was proposed in Wireframe/Design
> Reference when the feature request was made ?

While we should be doing this, the wireframes are more of a functional reference. They aren't meant to be taken literally as "this is exactly what the screen should look like". It seems we did make a mistake when developing the feature, and when reviewing, as it did not conform that closely to the wireframe, but the fact that it passed a wide design review during the meeting on Wednesday suggests that nobody was particularly concerned that it didn't match the wireframe.

> 
> 2 Why do you want to open two separate bugs(one for functionality and one
> for design) for a single feature request ?

Because it is finished. There are times when we need to move forward, not backwards. If the feature is done and has been approved by the project team it doesn't seem a good use of time to reopen it. We have a lot of features to implement and not a lot of time. Sometimes "good enough" is good enough. If you feel that the "design problem" is big enough, then you can open a bug on it and we can try to get to it if we can. You also have the option of opening said bug and working on it yourself, and then submitting a pull request.

> 
> 3 Why can't we spend a few more minutes to address the design issues rather
> than make a pull request with just the functionality working ?

If you had raised this during the review of that PR then it likely would have been addressed, but the feature is now complete and has been reviewed and approved by the team. At this point it doesn't make sense to reopen it. We need a new bug because it is done. This is not to say that we should always follow that pattern and just get things working without consideration for design (which is a bit of an exaggeration). We should endeavour to have the work we do also meet design requirements, but the time for discussing that is during the PR review, not after things have been merged and approved by the team. 

> 
> If it has been just about the fonts and how text is displayed it would have
> been a very minor issue but this one is about placement of elements on the
> page.
> 
> The elements in this page aren't where they need to be and the overall
> design is not consistent with rest of the pages of application.
> 
> I will leave it to Rebecca and Liz to review the design and :Osmose to
> comment on its consistency with rest of application pages.
> 
> This https://bug1020981.bugzilla.mozilla.org/attachment.cgi?id=8435535 is
> the current version and this
> https://bug1005114.bugzilla.mozilla.org/attachment.cgi?id=8421699 is what
> was proposed.
> 
> Please keep the following points in mind while giving your feedback:
> 1 The placement of user profile name.
> 2 The placement of pick a task button and edit profile button.
> 3 Two separate content-containers which seem to divide the page in two parts.

Comment 17

[Osmose [:osmose, :mkelly]]

Comment on attachment 8435535 [details]
user_profile.png

I'm not really the person to be reviewing design consistency for a feature. I abdicate responsibility!

Comment 18

[Rebecca Billings [:rbillings]]

1) Can we move this to a meeting or email discussion instead of taking over a bug? In the future if it's something that needs more than a comment or 2, offline seems preferable.

2) I care about the functional elements, not the exact placement of buttons. Concerns about design can be discussed during testing- and can be in separate bugs. We do not have a usability resource, but that's what we get with testing & we can also call in experts if we have a specific question. I do not have a problem with the task history and edit profile being separated from the dashboard that shows the task with a pick task button.

3) I'm fine with Maja's implementation of the design, even though it does not match the mockup 100% it does match the intention. Ideally it would have been specifically discussed that the mockups no longer matched- but it was not brought up during review. Again, if people don't like this it can be brought up in a bug/during testing and does not need to block development.

Comment 19

[Pankaj Malhotra (:bitgeeky)]

Comment on attachment 8434987 [details] [review]
Link to Github pull-request: https://github.com/mozilla/oneanddone/pull/125

Since most of us are fine with the new changes we can move forward.
Bob just a small suggestion on pull rest LGTM.

Comment 20

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8434987 [details] [review]
Link to Github pull-request: https://github.com/mozilla/oneanddone/pull/125

I think I addressed your comments, but I'm not sure. Please check and r?

Comment 21

[Pankaj Malhotra (:bitgeeky)]

Comment on attachment 8434987 [details] [review]
Link to Github pull-request: https://github.com/mozilla/oneanddone/pull/125

Looks good Bob :) just a very very tiny nit.

Comment 22

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8434987 [details] [review]
Link to Github pull-request: https://github.com/mozilla/oneanddone/pull/125

Landed in https://github.com/mozilla/oneanddone/commit/3c63a12b847a0e454921ea6fd3fc7322da16b648

Comment 23

[Rebecca Billings [:rbillings]]

Verified Edit Profile no longer displays on signed in front page view.