Store detail multiview

[D0nPiano]

ToDos

General

• building routes for all items in the tabs
• beeing more responsive.
• fix storeList, storeList map etc..
• use full screen width for mobile
• fix duplication of stores in desktop view when opening /group/1/stores
• rename Group -> GroupService

Routes

• change route names to:

 -> group
          -> groupDetail
                  -> description
                  -> members
                  -> stores
                  -> pickups
          -> store

• change directory structure according to routes
• add RouteTests

Nice-to-have

• highlight current store in StoreList
• add whiteframe to userPage
• group/*/description should only show one description on desktop
  -> what about editing the description?

Fixes

• fix tests
• fix mdMedia use in groupDetail

[D0nPiano]

Group

Store

Mobile

[tiltec]

I'll fix the tests later tonight :)

[tiltec]

I just have patchy online access, therefore I'll just dump my thoughts here while having a look at the code. Might do a more thorough review in the next days.

======

The route /group/6/description loads the description twice in desktop view.

Duplicated template code in group.html and its child components

Shouldn't the directory structure look more like this: (i.e. reflect the route)

group -> 
    groupDetail
    store
        -> storeDetail

We can have more than 1 component per module, that might make the module structure less bloated.

GroupDetailController: can $state.current.name change? Maybe this code belongs in a $doCheck or $onChanges function. To me, this code seems a bit confusing anyway, I don't understand why it is there.

groupDetail.html: there's a call to $mdMedia in the controller, but it's missing in the dependencies. Why doesn't the test fail?

groupDetail.js: why $state.groupData = group;? Ah, it's used by many child components. Well, global state is easy, but will it bite us in future?

Route tests are missing or don't work (storeDetail.spec.js). BTW, also userDetail misses a route test.

[tiltec]

I discovered some things that could/should be changed:

1. group module name is duplicated (group service and group component) -> rename group service module to groupService?
2. group-specific stuff is in group as well as in groupDetail -> move all group specifics (incl whole template) to groupDetail and make group state abstract?
3. createGroup is child of main state, but the URL is /group/create -> make createGroup a logical parent of group
4. storeDetail is a child of group, seems to be more fitting as child of group.groupDetail.stores?
5. the ui-view names mainView and detail are not really necessary, are they? I thought you need them when having multiple views per state

I did a lot of changes already, but right now nothing works anymore... so maybe I need to go back and start with smaller changes. What do you think about the points above?

[D0nPiano]

1. group module name is duplicated (group service and group component) -> rename group service module to groupService?

Sounds good to me!

2. group-specific stuff is in group as well as in groupDetail -> move all group specifics (incl whole template) to groupDetail and make group state abstract?
3. createGroup is child of main state, but the URL is /group/create -> make createGroup a logical parent of group

I think letting the group state resolve data for one specific group makes sense for URLs and data-wise (because both groupDetail and storeDetail need the data). group.create seems more clear, but really, what would the group state be doing then? It wouldn't resolve data anymore since group.create doesn't have data yet, and it also couldn't display a store list on the left anymore.

4. storeDetail is a child of group, seems to be more fitting as child of group.groupDetail.stores?

For me, group detail was more of a UI representation. So that:

• group: page with storeList on the left

• groupDetail: main page that shows the name & description

• and then a tabbed subviews of groupdetail (stores, members, pickups...)

Therefore group.store made more sense to me. But I think you also have a point. I think it just gets a little tricky with the ui-view elements, and it also makes the routes much longer. So I'd rather keep it the way it is. (probably goes along with the purposes of group and groupDetail generaly, so number 3 & 4)

5. the ui-view names mainView and detail are not really necessary, are they? I thought you need them when having multiple views per state

-> I'll look into that!

[tiltec]

@D0nPiano thanks for your quick reply :)

It's ok for me to leave points 2-4 as they are and work on 1 and 5. I had a call with @nicksellen a few days a ago and we came to the conclusion that this PR is mostly about improving navigation (e.g. breadcrumbs and page titles), so we should only add as much hierarchy in the code as necessary.

To me, the split between group and groupDetail is a bit confusing, but I could probably live with that.

[tiltec]

Store

Group

Group on mobile

Amazing work @D0nPiano ! Can't wait to have this public!

Just some more comments - they might be too nitpicky, so if it's too hard, no need to solve them right now.

How to best navigate between stores on mobile view? Using the back button?

The name of the group is mentioned 3 times close to the top bar (navbar, breadcrumbs, content title bar).. could we reduce that by merging the content title bar with the breadcrumbs?

The store list in desktop view is quite prominent (good to have the map visible, but takes much space from the main content list). I wonder if it's better to move it to the right.

[D0nPiano]

Thank you, glad you like it 🙂

...might be too nitpicky

meh, I like any kind of feedback :)

How to best navigate between stores on mobile view? Using the back button?

I think so. At least I couldn't think of a "clean way" yet. Do you have any ideas?

The name of the group is mentioned 3 times close to the top bar (navbar, breadcrumbs, content title bar).. could we reduce that by merging the content title bar with the breadcrumbs?

Like a subtitle? But yes, agreed. I think it's also way too prominent still. And I don't like the linebreaks on mobile. I'll look into that!

The store list in desktop view is quite prominent (good to have the map visible, but takes much space from the main content list). I wonder if it's better to move it to the right.

Agreed, especially on small desktop screens. Do you think just making it smaller also works for you? I'd like to keep it on the left somehow (I think sidebars are mostly on the left, and I always think that content-changing-buttons should be on the left of the changing content)

[tiltec]

Re: store sidebar
Maybe reducing the distance between the list and the left screen border to zero would help? And letting the main view float in the middle.