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

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ranbena, Assigned: evyatar)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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

Updated

5 years ago
Assignee: nobody → amirn
Blocks: 910302
(Reporter)

Updated

5 years ago
Status: NEW → ASSIGNED
(Reporter)

Updated

5 years ago
Assignee: amirn → evyatar
Evyatar, plz wait for final styling
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.
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.
Cristian, it's been decided to let users rename their collections.
Should this affect the manifest translation data?
Flags: needinfo?(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
Flags: needinfo?(crdlc)
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
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).
Here is implemented the new method to set custom names to the icons

https://github.com/crdlc/gaia/commit/ee9537eb30cb56a81b554e050760ba5053b16428
Depends on: 914115
Created attachment 812095 [details]
Patch - redirect to github PR
Attachment #812095 - Flags: review?(ran)
Attachment #812095 - Flags: review?(crdlc)
Got some comments on github
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
Flags: needinfo?(evyatar)
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
Attachment #812095 - Flags: review?(crdlc) → review-
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.
Flags: needinfo?(evyatar)
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?
Flags: needinfo?(evyatar)
it happened because I did input.blur() before removing the event listener (that made 'blur' do 'cancel')  - fixed.
Flags: needinfo?(evyatar)
(Reporter)

Updated

5 years ago
Attachment #812095 - Flags: review?(ran) → review+
(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
The feature works perfect on my device
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.
Attachment #812095 - Flags: review- → review+
merged to master
e787c882047b1e0c5bae4299499d3b529c755bd2

https://github.com/mozilla-b2g/gaia/commit/e787c882047b1e0c5bae4299499d3b529c755bd2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
blocking-b2g: --- → koi?
removing koi? since e.me 1.2 features are delayed for future version
blocking-b2g: koi? → ---
You need to log in before you can comment on or make changes to this bug.