sprintly / sprintly-kanban

A Kanban Board for Sprintly
https://kanban.sprint.ly
ISC License
494 stars 82 forks source link

Item wiring #4

Closed wookiehangover closed 9 years ago

wookiehangover commented 9 years ago

What does it do?

Wires up events on item cards, addressing additional styling concerns along the way, as well as wiring filters into the "load more" work that @florapdx landed. There's also work to wire up the pusher connection locally. We'll still need to set the correct channel name for production, but the bulk of the work and early edge cases are accounted for.

Where should the reviewer start?

There's a lot to look at here (sorry) but the best place to start is the ProductStore. There's UI and styling work in the item cards and column headers to take note of as well, but it's mostly incidental.

Any background context?

This one went a bit longer than expected, but there was a bit of a holding pattern with "Load More" landing, so it made sense to get some of the UI stuff in this branch before it came back in.

There's still test coverage that needs to be added to support the item wiring and pusher connection logic.

Also left are some loose ends w/r/t events on the cards (Move up, down, move multiple columns, etc) that need will need to be suppressed before release or otherwise wired up in a follow up PR.

Screenshots

Fixing wide-display issue screen shot 2015-03-23 at 9 24 40 am Updated item cards with text buttons and expanded details screen shot 2015-03-23 at 9 24 55 am Updated sorting dropdown screen shot 2015-03-23 at 9 25 34 am Use sprintly-ui for member search screen shot 2015-03-23 at 9 26 48 am

GIF?

florapdx commented 9 years ago

Yeah, I think this is almost there. The two things I'd like to talk about are item modification date management and the new item card styling. Where the former is concerned, I really just want to point out that it seems like there's a lot of dispersed logic reading/checking/setting 'last_modified' without a whole lot of documentation around why that's necessary. I also wonder if there's a more centralized way of handling this -- answer can totally be 'no', I just want to know more.

In terms of the new item card styling, I'm still really missing context on these changes. I have some strong concerns about the appearance of the new cards, specifically in that the card header area is so large that the information density on the page has been drastically reduced. We often hear from customers that they would like to see more density on the page, so it worries me that we've reduced the visible items per-column from somewhere in the 8-10 range to 5-6. I think this could be helped by reducing the size of the status-changer buttons, but I still have concerns about the aesthetics of the page, and am a little yellow on the fact that there was no (shared) discussion around these changes pre-PR. The sprintly logo is also so small that on my screen the letters don't appear fully formed :/

wookiehangover commented 9 years ago

I'm still really missing context on these changes

Happy to explain :)

The trouble with the card styles started when it was time to wire the status change buttons. With the left aligned circular controls, we were relying on icons to indicate the button actions, and only showing one available action. This is problematic because we don't have an icon system for these status that "works" without additional text to explain it, and it was difficult keeping status icons-to-column-status straight.

The other issue that came out of spending more time with the item card actions was that we lose the secondary action on hover from the sprint.ly item cards, so we needed another button. So two buttons were needed, along with getting the sizing to match on our circular controls. That's a lot to do in the left aligned header.

After discussing with Jace on Thursday afternoon when I ran into these issues, I rearranged the item cards to be more of an analog of what we currently have on sprint.ly, and I shared the results as soon as possible on Friday.

[...] the card header area is so large that the information density on the page has been drastically reduced

The header only makes the card about 16px "fatter"

screen shot 2015-03-23 at 11 08 57 am

and that can be trimmed down by using a smaller size that default on the buttons.

florapdx commented 9 years ago

Yeah, I guess that I had thought that it was still under discussion whether or not we were going to keep those buttons at all (and my impression given the discussions we'd been having w/in sprintly were that it's redundant to have both the buttons and drag-and-drop, and that we were generally in favor of the latter b/c of the ability to do both intra-column and inter-column organization with one tool).

I don't have any real attachment to the old layout/design, btw, I just fell like the card header needs to be a little slimmer so we can fit a bit more on the page and it doesn't appear like there's so much whitespace (maybe reintro the estimator color too? not sure about that, but it's very 'pastel' without that stronger color. What do you think?)