sprintly / sprintly-kanban

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

Fix product and filters init #6

Closed wookiehangover closed 9 years ago

wookiehangover commented 9 years ago

What does it do?

Fixes product initialization issues that were preventing items from moving between columns after switching between routes.

Where should the reviewer start?

Most of the work is in the ProductStore and FilterStore, with some clean up happening in the Items view-controller.

How should this be manually tested?

Load a product and upgrade an item's status. The card should move to the next column. Load another product through the products dropdown menu and upgrade a card once it loads. The card should move between columns as expected.

Background context?

Logic that had been in the Items view-controller has been moved into the ProductStore, simplifying the Items component initialization.

Doing so exposed a problem with Filters initialization not being idempotent: the members for a given product need to be loaded fresh every time you switch products.

Relevant tickets

https://sprint.ly/product/1/item/9505

GIF?

tdu2

florapdx commented 9 years ago

Haven't looked at code yet, but just from using, this is looking really good! Two minor issues and one question:

wookiehangover commented 9 years ago

assignee menu in "Add filter" menu doesn't update for me when I change products

Looking into that.

there's kind of a funny border on the right and bottom edges of the product chooser menu

That's really odd and I haven't seen it / can't reproduce it locally (chrome / ff nightly)

question: not sure if the move up/down/bottom/top buttons are wired in yet? guessing not yet? Clicking these sends me back to the product selection page.

Those aren't wired in yet and need API endpoints https://sprint.ly/product/1/item/9574... I have another PR for when that lands.

wookiehangover commented 9 years ago

there's kind of a funny border on the right and bottom edges of the product chooser menu

That's really odd and I haven't seen it / can't reproduce it locally (chrome / ff nightly)

@florapdx Ok, looking again I think those are browser chrome scroll-bars... weird that they're showing up for you. Sometimes OS X can get into a funky state and force them on, but they're usually off by default these days. Is this like an all the time thing for you?

wookiehangover commented 9 years ago

https://github.com/sprintly/sprintly-ui/pull/12 along the rest of the commits I just pushed address the issue with the selector menu losing state when moving between products.

florapdx commented 9 years ago

@wookiehangover Seeing a couple of issues with this still:

The first is that the header filter menu is borked: screen shot 2015-04-01 at 4 55 26 pm

The second is that after you've removed an assignee filter, the assignee label persists as if active in the filter menu (this is after deselecting Alex from the filter dropdown): screen shot 2015-04-01 at 4 55 15 pm

Here's a recordit of what I did just in case the flow is unclear: http://g.recordit.co/PRgHQ4Oe7V.gif

florapdx commented 9 years ago

Also still seeing the same carryover of members when switching products, so.....hmmm. Reinstalled everything and it works! (above issues still stand, though, unfortunately.)

One thing: You've still got sprintly-ui v1.0.0 in package.json, but the newer version with your menu fix is v1.0.1, so that will need to be updated :)

wookiehangover commented 9 years ago

Ok, I've got the labels thing and the missing styles addressed. Thanks for posting the recordit.

http://recordit.co/7bmEfAgEYR

And the version in the package.json is declared as ^1.0.0, so we automatically pick up all MINOR and PATCH releases.

justinabrahms commented 9 years ago

If upgrade status fails, we should rectify the UI somehow.

Have this bug specifically logged at https://sprint.ly/product/1/item/9597 but we should be more resillient to errors.

Okay if this happens in a separate PR. If you go that route, file a new bug and feel free to merge this. :+1: