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

RFC: Resource view UI polish #3961

Open
Lindsayauchin opened this issue Jun 2, 2019 · 10 comments
Open

RFC: Resource view UI polish #3961

Lindsayauchin opened this issue Jun 2, 2019 · 10 comments

Comments

@Lindsayauchin
Copy link
Contributor

Lindsayauchin commented Jun 2, 2019

Now that Users have used the pinning feature for sometime, the first version of this implementation needs some UX iteration and UI polish.

Recommendations:

  1. Update Comment overlay
  1. Remove pin bar in L2 nav area
  1. Dismissing comment overlay
  1. Editing comment
  1. No overlay by default

6 Update icon disable/enable button in version list

  1. UI polish on the commenting input overlay

  2. Layout "re-gridding" the table to an 8px grid

  1. Display meta data on a version
    -make issue

Related open issues

#4211
#3960

Current State:

Screen Shot 2019-06-02 at 2 41 43 PM

Screen Shot 2019-11-11 at 10 00 09 AM

Proposed State:

Screen Shot 2019-11-13 at 10 00 04 AM

@jamieklassen
Copy link
Member

@kcmannem suggests that we have a 'sticky' row at the top of the versions list for the version to which the resource is pinned.

@Lindsayauchin
Copy link
Contributor Author

Lindsayauchin commented Oct 24, 2019

Updated Design Mock:

For visibility design iteration based on the feedback in IPM.

Display the currently pinned resource sticky to the top of the version list.

  • Remove tooltip as truncation should not be needed in this area. Bar should expand by one row if needed.

Display pinned comment in the same top bar

  • timestamp and user name + comment. Bar should expand by one row if needed.

Screen Shot 2019-10-28 at 10 31 20 AM

@jamieklassen
Copy link
Member

jamieklassen commented Oct 31, 2019

@Lindsayauchin what happens to the comment bar - is it still at the bottom? if not, what does editing the pin comment look like?

If it is still at the bottom, then let's focus the scope of this issue to just be changing the 'pin bar' to a 'sticky version', and not add the comment to that sticky version.

@jomsie
Copy link

jomsie commented Oct 31, 2019

Talked to @Lindsayauchin @matthewpereira and @kcmannem regarding our questions, and came to these decisions:

  • Pin icon in the currently pinned bar (at the top) is only a label now, no action (so change bg colour)
  • Clicking on the pinned resource version (ref in this case) scrolls to the pinned resource in the list
  • Clicking on the pin comment in the currently pinned bar brings up/shows the pinned comment edit section
  • Pinning actions only happen in the list

annotation-time

@Lindsayauchin feel free to add any comments I missed!

@jomsie
Copy link

jomsie commented Oct 31, 2019

Just my own comment: I'm thinking about how obvious it will be to know clicking on the ref 12dcf... brings you to the pinned version in the list...

My thought is people might not realize that, and since they can't unpin from the top bar anymore, they'll start searching through pages to unpin. Should we remove the ability to unpin from that top bar?

@jamieklassen
Copy link
Member

Versions are paginated, and for the moment the API doesn't tell you which page the pinned version lives on (if any). The current implementation has meant that pages of versions are only fetched on clicking the pagination chevrons. The proposed changes seem to mandate eagerly fetching versions in order to find the pinned one

@jomsie
Copy link

jomsie commented Oct 31, 2019

Good point, that seems like a pretty big undertaking (either on our side or API side). @Lindsayauchin how attached are you to the "click resource version to find it in the list" thing?

@matthewpereira
Copy link

matthewpereira commented Nov 1, 2019

Side topic of interest: it came up in conversation when we were looking at this on screen that the pin icon can be difficult to make out, and might be even more difficult on screens with a lower DPI than what we have in the office. I whipped up a couple quick variations to provide a starting point for discussion.

Each row has a different icon for comparison (you might have to click on each image and view them at full size to see the full effect):

Frame 1

Frame 2

Frame 3

From top to bottom:

  1. Original icon
  2. Fattened icon
  3. Resized and adjusted, offset so that the pin is lined up with the pixel grid
  4. Resized and adjusted, centered

Option 3 is nicest on ~72dpi - 96dpi displays (eg, lots of Windows laptops, non-Retina Mac models), while Option 4 seems nicest on high DPI displays.

View the original mockup on Figma

Maybe we could swap out the svg in an upcoming release?

pin icon - resized offset.svg.zip
pin icon - resized centered.svg.zip

@vito
Copy link
Member

vito commented Nov 4, 2019

while playing with button sizes/spacing and icon style/alignment, maybe we can get a solution for #4211 as well.

Agreed that the pin button on the "sticky" version will still unpin, but clicking the version will not do any scrolling.

hitting 'Escape' with the textarea focused seems like it should dismiss the comment bar.
@Lindsayauchin pls weigh in on the 'dismissing' story.

@Lindsayauchin
Copy link
Contributor Author

Lindsayauchin commented Nov 13, 2019

Sketch to talk about in IPM today with the team:

Screen Shot 2019-11-13 at 10 00 04 AM

@Lindsayauchin Lindsayauchin changed the title Resource view: Resource Pinning Polish RFC: Resource view UI polish Nov 13, 2019
@Lindsayauchin Lindsayauchin self-assigned this Nov 14, 2019
@vito vito added this to To do in Resource View UI Polish Dec 2, 2019
@vito vito moved this from To do to End goals in Resource View UI Polish Dec 2, 2019
@jamieklassen jamieklassen moved this from End goals to In progress in Resource View UI Polish Mar 17, 2020
@xtremerui xtremerui removed their assignment Mar 17, 2020
@muntac muntac assigned taylorsilva and unassigned taylorsilva Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests