Card reference ux - Githubissues

discorick commented 8 years ago

Merges into https://github.com/huboard/huboard-web/pull/327

I manually merged in #309 so I could use actual references to test with.

discorick commented 8 years ago

Blocked by: https://github.com/huboard/huboard-web/pull/335, waiting for it to be merged to bring in the new blocked/ready state code.

dahlbyk commented 8 years ago

Only returns a single instance of any given reference

Test issue with repeated references: https://huboard-rails-pr-338.herokuapp.com/huboard-testing/card-designs-1#/issues/165129162

Hadn't thought of this before, but we should probably include title references: https://huboard-rails-pr-338.herokuapp.com/huboard-testing/card-designs-1#/issues/167947120

discorick commented 8 years ago

Hadn't thought of this before, but we should probably include title references

No sure I agree on that one, not even GitHub supports this.

dahlbyk commented 8 years ago

Hadn't thought of this before, but we should probably include title references

No sure I agree on that one, not even GitHub supports this.

I swear they did... Nevermind, then.

Anything else here, or are you ready for code review?

discorick commented 8 years ago

Yeah ready for code review!

dahlbyk commented 8 years ago

Moves closed and archive trays below references

Not sure about this change... I think of the state/action trays as part of the card and the references as "extra information".

discorick commented 8 years ago

As a comparison:

I lean towards the second, where the tray floats to the bottom for a couple of reasons:

When I glance of a busy board, I can mistake the 'lighter' styling of the references for their own cards, which I think has to do with the stark contrast of the state trays
In the same line of thinking, state trays look 'heavier' than references, seems more naturally for them to sink to the bottom

@jackycute, curious your thoughts?

jackycute commented 8 years ago

For consistency and the tray color as divider, I think keep the tray to the bottom is better.

dahlbyk commented 8 years ago

Concerns about using the tray as a divider seem to apply regardless, since most cards won't have a ready/blocked/done tray. I can convince myself that putting the tray above or below references is better, but if we're concerned about separating the cards we should add more margin or a darker border around cards.

discorick commented 8 years ago

To clarify, I am not so much concerned about cards not looking separated, it was more when a status is present it makes one card look like 2 from the peripherals

discorick commented 8 years ago

@dahlbyk @rauhryan , I think this is ready to ship, unless we want to wait for tray icons ?

dahlbyk commented 8 years ago

unless we want to wait for tray icons ?

Not sure what you mean by this?

discorick commented 8 years ago

unless we want to wait for tray icons ?

Not sure what you mean by this?

In the mockup I was given, there was a checkmark on the left side of the 'closed' tray:

Was waiting on @rauhryan for the asset.

dahlbyk commented 8 years ago

Not really card references, but since you're fiddling with the tray UX...

We currently suppress Ready/Blocked for closed cards, but not for open cards in the last column. Showing two trays is weird, so we should pick a winner. I'm ambivalent, but inclined to respect Ready/Blocked and hide the Done+Close tray. https://huboard-rails-pr-338.herokuapp.com/huboard-testing/spooky-octo-pug#/

discorick commented 8 years ago

This is true, we could hide them, they definitely look weird. The flip side is setting a card in the final column to blocked/ready would be unconventional to begin with (other than with labels it's disabled) . If we were going to pick a winner it would be disable blocked/ready state trays.

dahlbyk commented 8 years ago

The flip side is setting a card in the final column to blocked/ready would be unconventional to begin with (other than with labels it's disabled) . If we were going to pick a winner it would be disable blocked/ready state trays.

It's absolutely unconventional, but still a state we need to handle. Moving into the last column within HB will always remove the blocked/ready state, but we don't disable the toggles in the fullscreen issue view until the issue has been closed.

Ultimately my leaning toward respecting ready/blocked comes down to the fact that we have additional information that's 1) easy for the user to remove if it's no longer applicable, and 2) unlikely to be present without explicit action having been taken within the app.

We could also split the difference and add a Close button to blocked/ready trays if they're in the last column.

dahlbyk commented 8 years ago

FWIW, current UX for blocked/done in last column:

rauhryan commented 8 years ago

We aren't showing issues that are closed over 2 weeks ago, I'd expect it to be shown as a closed reference. Probably adds some complexity and extra work around the opening of the issue because it's not in the client side model yet.
Poking around and using it, it feels like it's missing the title of the issue. Not blocking, but it having it done and working exposes flaws in the design.

dahlbyk commented 8 years ago

Poking around and using it, it feels like it's missing the title of the issue. Not blocking, but it having it done and working exposes flaws in the design.

The reference rows are missing the title, you mean? Should we show it on hover or something, like assignees? I can't see it working with the extra text visible at all times.

discorick commented 8 years ago

We aren't showing issues that are closed over 2 weeks ago, I'd expect it to be shown as a closed reference. Probably adds some complexity and extra work around the opening of the issue because it's not in the client side model yet.

Initially I had that implemented, but the references felt very stale and uninformative. They cannot be clicked and shown as fullscreen issues, we can't tell if they are PR's or not - no comment count is available. Removing them improved vertical space a good deal as well.

dahlbyk commented 8 years ago

We aren't showing issues that are closed over 2 weeks ago, I'd expect it to be shown as a closed reference. Probably adds some complexity and extra work around the opening of the issue because it's not in the client side model yet.

Initially I had that implemented, but the references felt very stale and uninformative. They cannot be clicked and shown as fullscreen issues, we can't tell if they are PR's or not - no comment count is available. Removing them improved vertical space a good deal as well.

In theory we could build a queue of closed issues to fetch one-off. Or we could add a "sync" type button on them to fetch details on demand.

Why can't we show them full-screen? We should have owner/repo and id, which should be enough to fetch details as needed?

All that said, I'm fine omitting them for now. If we notice, or get feedback, that their absence is problematic, we can spend some time figuring out the right UI.

discorick commented 8 years ago

Why can't we show them full-screen? We should have owner/repo and id, which should be enough to fetch details as needed?

Actually yeah thats true, if we want to do this later its going to be possible.

@dahlbyk @rauhryan can we merge this guy in ?

dahlbyk commented 8 years ago

@dahlbyk @rauhryan can we merge this guy in ?

Almost. Just noticed a mismatch in the linked repo border width:

Also, can you make a note in the parent that we need to resolve https://github.com/huboard/huboard-web/pull/338#issuecomment-237854443?

discorick commented 8 years ago

Almost. Just noticed a mismatch in the linked repo border width:

I left that intentionally, the full "linked" border along the side of the references looked a little weird to me visually, also not all references are linked . I can paste a side by side if you like?

Also, can you make a note in the parent that we need to resolve #338 (comment)?

Done!

rauhryan commented 8 years ago

:shipit:

huboard / huboard-web

Card reference ux #338