concourse / concourse

Concourse is a container-based continuous thing-doer written in Go.
https://concourse-ci.org
Apache License 2.0
7.42k stars 848 forks source link

Sidebar feedback round 2 #3872

Closed clarafu closed 5 years ago

clarafu commented 5 years ago

The new sidebar looks great!!

Feedback 1: It might help to use a different colour (maybe darker) font for the team names within the sidebar so that at a quick glance it's easy to differentiate between the teams and pipelines?

Screen Shot 2019-05-15 at 9 33 36 PM

Feedback 2: j/k keybindings for switching between pipelines would be diggity dope.

Feedback 3: Drag to reorder would be nice but maybe that should be a separate issue?

Thx!!

@vito and @clarafu

jamieklassen commented 5 years ago

Love these ideas. Mildly concerned about overloading j/k on the build page, unless there's a hotkey to focus/expand the sidebar too...

StevenArmstrong commented 5 years ago

Having deployed 5.2 yesterday the two main features that would really enhance useability would be a search filter at the top of the sidebar like the dashboard view that could generate a list of matching pipelines based on regex. Also nesting pipeline groups as a collapsable links under a pipeline. So the drill through is team -> pipeline -> pipeline groups would also be awesome.

clarafu commented 5 years ago

@StevenArmstrong Those are great ideas!

@pivotal-jamie-klassen Oh I totally didn't know j/k was used in the build page already, you're right it would only make sense if there was a hotkey for opening the sidebar.

jamieklassen commented 5 years ago

@vito I don't recall where we settled for the scope of this. Which of the following do we expect to achieve during the course of this issue?

  1. [ ] Color of team vs pipeline text in sidebar
  2. [ ] keyboard shortcuts
  3. [ ] drag to reorder
  4. [ ] drill into groups
  5. [ ] filter input
Lindsayauchin commented 5 years ago

UI Feedback:

  1. Active State team name:

    • [x] no opacity on any element
  2. Active State pipeline:

  1. Hover:

    • [x] opacity of the container should be 40%
    • [x] pipeline name, team name
  2. Inactive pipeline name

    • [x] reduce the opacity of the pipeline names to 30%
  3. Inactive team name

    • [x] keep team names at 60% opacity
  4. Active toggle on team name

    • [x] chevron at 70% opacity when the team expanded, 100% when there is a pipeline selected in that list

Screen Shot 2019-05-22 at 6 03 22 PM

Additional:

Current:

Screen Shot 2019-05-22 at 4 05 16 PM

Expected:

Screen Shot 2019-05-22 at 6 20 16 PM

jomsie commented 5 years ago

@Lindsayauchin Hi! You have a task outline of the container should be: but didn't put in a colour. I looked at the screenshot and it looks like it should be #979797. Let me know if that's not right!

jamieklassen commented 5 years ago

For

only display tooltip on long names that are truncated

here's what I'm thinking:

  1. Add id attributes to each of the team names and pipeline names in the sidebar. This will involve one of two strategies:
    • create an injective function DomID -> String that generates valid HTML id strings (see below). This could in principle be reused for any element that has event listeners, including clicks and inputs, etc.
    • create two injective functions, PipelineIdentifier -> String for the sidebar pipelines and and String -> String for the teams, that both generate valid HTML id strings. Narrower scope, but also less reusable.
  2. Add a new Effect that calls getViewportOf with the above IDs, and the corresponding Callback just contains the relevant Result Viewport of the element in question.
  3. Any time the list of pipelines might change (which is probably just whenever Application.handleCallback (PipelinesFetched _) happens), run the above effect for each sidebar team and pipeline.
  4. Referring to https://github.com/concourse/concourse/blob/master/web/public/index.html#L164, handle the abovementioned Result View callback as follows: ensure that the title attribute only gets set on those sidebar teams and pipelines for which offsetWidth < scrollWidth.

potential concern

When the pipelines refresh, the view will re-render, but at maybe the same time, the side effects that ask the browser for the viewports for each of the sidebar entities will run. I'm not sure exactly how the Elm runtime will sequence rendering the view and running the commands, so if there is a race condition here let's figure out a decent way to exercise it, or carefully study the Elm source code to be confident.

improvements

  1. Bring existing dashboard functionality in line with the above changes
    • Refactor these to use the Effect described above instead of a port.
    • Convert the dashboard tooltips to use inline styles; cover them with tests.
  2. Extract some kind of reusable tooltip functionality from the above, and use it instead of the title attribute on the sidebar teams and pipelines.

notes

HTML ID strings:

must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods (".").

https://www.w3.org/TR/html4/types.html#type-id

jomsie commented 5 years ago

@pivotal-jamie-klassen just a quick question, looks like html5 id spec stipulates that "The value must be unique amongst all the IDs in the element's home subtree and must contain at least one character. The value must not contain any space characters."

https://www.w3.org/TR/2011/WD-html5-20110525/elements.html#the-id-attribute

we're good with good ol' html5, right?

jomsie commented 5 years ago

With our current implementation, you can click/hover on the team icon and some action happens, but hovering/clicking the pipeline icon does nothing. (Mostly a note for myself to follow up with @Lindsayauchin)

jomsie commented 5 years ago

As it stands, the sidebar overflow team/pipeline names use the title attribute. I'm starting to think there should be a separate issue for:

  1. switching the data-tooltip styles to be inline. (currently they are in web/assets/css/dashboard/less)
  2. switching the sidebar title title to data-tooltip.
jomsie commented 5 years ago

@pivotal-jamie-klassen some of your fears (or potential concerns) might be true with the current implementation... with lots of big ol' pipeline names and hovering across many of them REALLY quick, we get this:

Screen Shot 2019-06-18 at 3 28 16 PM

the text updated properly, but the hover isn't on the proper name.

this comment is partially just letting you know, and partially making sure i don't forget about it!

jomsie commented 5 years ago

Just talked to @Lindsayauchin and there are a couple more changes to make

Additional (maybe break into a new issue) Scrollbar: