1025963 - [Collection app] Pinned apps edit mode

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

[Ran Ben Aharon [:ranbena] EverythingMe]

When long pressing pinned apps, they should enter edit mode (able to be removed and dragged).

Comment 1

[Ran Ben Aharon [:ranbena] EverythingMe]

Hey Peter, we're going to need UX guidance here.

In v1.3 Collections, apps would pulsate. In home2, the background changes and a title comes in from the top.

How should it look/behave in Collections? Note that only part of the apps (above the divider) are editable.

Comment 2

[Ran Ben Aharon [:ranbena] EverythingMe]

Kevin, this requires enabling edit mode on a subset of items in gaia-grid. Is this technically possible?

Comment 3

[Kevin Grandon :kgrandon]

Personally I would recommend a different approach due to the homescreen edit mode changes. We can allow a subset of the items to be draggable, but I would recommend changing it a bit to allow a one way drag to the pinned section (to allow pinning via dragging). The code isn't there for dragdrop subsets yet, but we can add it this week.

We probably want to try to use a similar UX if we can, so no pulsing and an edit header.

Comment 4

[Ran Ben Aharon [:ranbena] EverythingMe]

How about hiding the undraggable items in collection edit mode?
1. Won't require draggable subset work.
2. Won't require having special UI - can look just like the homescreen.

Comment 5

[Kevin Grandon :kgrandon]

I think that's fine by me.

Comment 6

[Ran Ben Aharon [:ranbena] EverythingMe]

I think it should be a gaia-grid feature - when in edit mode - hide uneditable items.
Wdyt Kevin?

Comment 7

[Ran Ben Aharon [:ranbena] EverythingMe]

Would you rather keep it a Collection feature?

Comment 8

[Kevin Grandon :kgrandon]

(In reply to Ran Ben Aharon (Everything.me) from comment #6)
> I think it should be a gaia-grid feature - when in edit mode - hide
> uneditable items.
> Wdyt Kevin?

This sounds fine to me. We'll probably want to add a new property, 'draggable', or 'canDrag' for example. When we enter edit mode we can hide everything else.

Comment 9

[Ran Ben Aharon [:ranbena] EverythingMe]

Ok, I'll take a swing at it

Comment 10

[Ran Ben Aharon [:ranbena] EverythingMe]

Peter, I think we figured it out

Comment 11

[Jason Smith [:jsmith]]

Looks like this is required feature work for collections.

Candice - Can you add the feature-b2g flag here?

Comment 12

[Ran Ben Aharon [:ranbena] EverythingMe]

Attachment: WIP 1

Comment 13

[Ran Ben Aharon [:ranbena] EverythingMe]

WIP 1 features:
* Long press a pinned app - transitions to edit mode
* All pinned apps are removable
* Non-pinned web results are undraggable and "muted"
* New sorting gets saved to db

Comment 14

[Ran Ben Aharon [:ranbena] EverythingMe]

Kevin
1. I had to change some stuff up in grid items in order to toggle draggable and removable.
2. Clicking the home button exits the app instead of edit mode. Do you think is feasible? Is it crucial?
3. It's possible to drag pinned apps beyond web results. We're gonna need to disable that option. Can you help out with that?

Thanks, Ran

Comment 15

[Kevin Grandon :kgrandon]

(In reply to Ran Ben Aharon (Everything.me) from comment #14)
> Kevin
> 1. I had to change some stuff up in grid items in order to toggle draggable
> and removable.

Sounds good - augmenting the grid for feature work will likely be a common thing.


> 2. Clicking the home button exits the app instead of edit mode. Do you think
> is feasible? Is it crucial?

We can fix this, though it's probably going to be some system work, so if you guys are ok with it - let's do it in a follow-up. We have the done button in the edit header, so that should be fine for now.


> 3. It's possible to drag pinned apps beyond web results. We're gonna need to
> disable that option. Can you help out with that?

Sure.. we may want to make it so that if you would drop the item that would cause it to go anywhere past the last pinned item, we just insert it at the end instead. I think that's probably going to be the easiest solution.

Comment 16

[Ran Ben Aharon [:ranbena] EverythingMe]

Kevin, I've updated the WIP branch.

* Notice there are 4 commits for convenience.

* Dragging over web results (and on empty spots aside them) is unsolved. Basically, disabling placeholders solves the problem but presents another - placeholders are required for this to work and not overshoot array index https://github.com/EverythingMe/gaia/blob/1025963-edit-mode/shared/elements/gaia_grid/js/grid_view.js#L326

* I assume we can leave the homebutton issue as is for now.

You can either take this or simply hint me on a possible solution.

Comment 17

[Kevin Grandon :kgrandon]

I am going to steal if that is ok.

Comment 18

[Kevin Grandon :kgrandon]

Attachment: Pull request

This is a rebased and updated pull request.

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 8443133 [details] [review]
Pull request

Hey guys - 

This is a hijacked version of Ran's work to have edit mode for pinned results in collections - and with an integration test!

I'm pretty happy with the integration tests. It pins two web results, enters edit mode, and swaps their positions. I'd love to get this landed by tomorrow, let me know what you think.

Comment 20

[Kevin Grandon :kgrandon]

Comment on attachment 8443133 [details] [review]
Pull request

Cristian is around right now, so if you can take a look I would appreciate it.

Comment 21

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8443133 [details] [review]
Pull request

The code looks good to me :) Good job as usual

Comment 22

[Kevin Grandon :kgrandon]

Comment on attachment 8443133 [details] [review]
Pull request

Thanks a lot Cristian. In order to move fast, I'm going to go with Cristian's R+ here. If you guys have any comments at all, feel free to leave them. I have to rebase anyway, so it's going to be a while before this lands.

Comment 23

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/7734b891710170a76e5b9ba78d37dd42194a4cd7

Comment 24

[Kevin Grandon :kgrandon]

Attachment: Follow-up commit (also uplift)

Landed a follow-up commit because there was one removable change I forgot to take from Ran's pull request.

Comment 25

[Kevin Grandon :kgrandon]

Follow-up commit changeset: https://github.com/mozilla-b2g/gaia/commit/a07a5d7f5634b477284d31d6f09fc0ec9def49c0

Comment 26

[Johan Lorenzo [:jlorenzo]]

Long press: OK
Drag: OK
Delete: OK

Comment 27

[Kevin Grandon :kgrandon]

Comment on attachment 8443133 [details] [review]
Pull request

Needed for the vertical homescreen and late l10n.

Comment 28

[Kevin Grandon :kgrandon]

2.0:
https://github.com/mozilla-b2g/gaia/commit/be9e6ff63dbcd0bb0896382b885380a960eb1a1e
https://github.com/mozilla-b2g/gaia/commit/d87b77cc0bc0e70f2f559f697d26f21bb55a771e

Comment 29

[Amir Nissim (:amirn)]

The Collection app crashes when I long tap a pinned result. Can't get into edit mode.
Does it work for you? Do we have a test for this?

Comment 30

[Kevin Grandon :kgrandon]

(In reply to Amir Nissim (:amirn) from comment #29)
> The Collection app crashes when I long tap a pinned result. Can't get into
> edit mode.
> Does it work for you? Do we have a test for this?

Strange, it should work - it's something that a lot of people are testing. What device? Anything in logcat?

Comment 31

[Amir Nissim (:amirn)]

Flame, nothing in logcat unfortunately :(

Comment 32

[Kevin Grandon :kgrandon]

(In reply to Amir Nissim (:amirn) from comment #31)
> Flame, nothing in logcat unfortunately :(

Hmm, strange. And you tried doing a reset-gaia and other collections? I haven't seen it or heard reports of this yet =/

Comment 33

[Amir Nissim (:amirn)]

false alarm, flashing a new gecko fixed it. sorry!