911550 - [e.me][feature] Enable Collection title "edit in place"

Status: RESOLVED FIXED

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

Description

[Ran Ben Aharon [:ranbena] EverythingMe]

When Collection is open, tapping it should enable "edit in place". Tapping anywhere out of it, toggled the edit mode, and applies the change.

Comment 1

[Ran Ben Aharon [:ranbena] EverythingMe]

Evyatar, plz wait for final styling

Comment 2

[Ran Ben Aharon [:ranbena] EverythingMe]

Final style will probably not be in before Sunday so here's the gist.
1. The proposed method has been agreed upon.
2. Instead of a back button in edit mode, an X should appear which cancels edit mode.
3. In edit mode a modal should appear over results. Clicking on it cancels edit mode.

Comment 3

[Evyatar 'Tron' Amitay (everything.me)]

Branch: *collection-rename*
Commit: 75d5793eb02a1e84fd48dabf9f922652d3f61ee9

https://github.com/EverythingMe/gaia/commit/75d5793eb02a1e84fd48dabf9f922652d3f61ee9


I'm not sure about the change on page.js.
The issue is that we need to update the icon's label. currently calling the icon's update method only updates the icon - since in translate() there's a condition to NOT translate Collections or Bookmarks.

Comment 4

[Ran Ben Aharon [:ranbena] EverythingMe]

Cristian, it's been decided to let users rename their collections.
Should this affect the manifest translation data?

Comment 5

[Cristian Rodriguez (:crdlc)]

As said during the first meeting on Monday, it should be very easy to implement, I can do it in the bug 911959. Thanks a lot

Comment 6

[Ran Ben Aharon [:ranbena] EverythingMe]

So Evyatar, can you prepare the UI changes in the meantime?
Eventually, we'll probably have a GridManager collection rename method that replaces the homescreen text, saves to DB and defines that collection is non-translationable

Comment 7

[Cristian Rodriguez (:crdlc)]

I will provide a method call setName for Icon objects. If third-parties set this name, the icons will be updated and will save that info in descriptor.customName. So if there is a translation in the UI and we are a collection and the customName is defined it won't be translated anymore (non-translationable).

Comment 8

[Cristian Rodriguez (:crdlc)]

Here is implemented the new method to set custom names to the icons

https://github.com/crdlc/gaia/commit/ee9537eb30cb56a81b554e050760ba5053b16428

Comment 9

[Evyatar 'Tron' Amitay (everything.me)]

Attachment: Patch - redirect to github PR

Comment 10

[Ran Ben Aharon [:ranbena] EverythingMe]

Got some comments on github

Comment 11

[Ran Ben Aharon [:ranbena] EverythingMe]

Evyatar, I would like to make sure this works on manually added Collections before merging it in.
You can do this on the "save search" feature branch Bug 911022

Comment 12

[Cristian Rodriguez (:crdlc)]

Comment on attachment 812095 [details]
Patch - redirect to github PR

Some concerns:

1. Bug title: "Re-name collections". Why was setIcon code added?

2. If the icon was modified, call install and it will be updated. Please don't open more paths to the homescreen code

Comment 13

[Evyatar 'Tron' Amitay (everything.me)]

setIcon was added since currently the only way to update an icon on the grid is to call the install. The install function replaces the ENTIRE descriptor.

We have 2 distinct cases - changing the icon, and changing the name. We don't want to replace the entire descriptor if we only change the name, or if we only change the icon.

If we keep a single "install" function - we need to save ALL of the collection data in e.me code as well. for example, when updating the icon we need to send the customName as well.

Comment 14

[Ran Ben Aharon [:ranbena] EverythingMe]

One last thing on my behalf - after confirming name change, the previous name shows and then immediately changes to the new name. Can you take care of that?

Comment 15

[Evyatar 'Tron' Amitay (everything.me)]

it happened because I did input.blur() before removing the event listener (that made 'blur' do 'cancel')  - fixed.

Comment 16

[Cristian Rodriguez (:crdlc)]

(In reply to Evyatar 'Tron' Amitay (everything.me) from comment #13)
> setIcon was added since currently the only way to update an icon on the grid
> is to call the install. The install function replaces the ENTIRE descriptor.
> 
> We have 2 distinct cases - changing the icon, and changing the name. We
> don't want to replace the entire descriptor if we only change the name, or
> if we only change the icon.
> 
> If we keep a single "install" function - we need to save ALL of the
> collection data in e.me code as well. for example, when updating the icon we
> need to send the customName as well.

I agree and I am comfortable with it but my concern is why we add setImage method to page.js when the title of the bug is Enable Collection title "edit in place". Or the title is wrong or that code should be in other pr IMHO

Comment 17

[Cristian Rodriguez (:crdlc)]

The feature works perfect on my device

Comment 18

[Evyatar 'Tron' Amitay (everything.me)]

re the setImage - unfortunately it was required here, cause the alternative meant adding customName throughout our code and making the PR even larger... This method is just a side effect of the changes needed for this PR.

Comment 19

[Evyatar 'Tron' Amitay (everything.me)]

merged to master
e787c882047b1e0c5bae4299499d3b529c755bd2

https://github.com/mozilla-b2g/gaia/commit/e787c882047b1e0c5bae4299499d3b529c755bd2

Comment 20

[Evyatar 'Tron' Amitay (everything.me)]

removing koi? since e.me 1.2 features are delayed for future version