Closed
Bug 1144849
Opened 10 years ago
Closed 10 years ago
[Add-On Manager] Support renaming add-ons in the activity detail view
Categories
(Firefox OS Graveyard :: Gaia, defect, P1)
Tracking
(b2g-master unaffected)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
b2g-master | --- | unaffected |
People
(Reporter: justindarc, Assigned: yzen, NeedInfo)
References
Details
(Whiteboard: [spark])
Attachments
(4 files, 1 obsolete file)
Add-ons should be able to be renamed when viewed coming in directly from an activity (Bug 1144848).
Reporter | ||
Updated•10 years ago
|
Comment 1•10 years ago
|
||
This will not go on master.
Assignee: jdarcangelo → yzenevich
No longer blocks: 1144848
status-b2g-master:
--- → unaffected
Priority: P2 → P1
See Also: → 1144848
Comment 2•10 years ago
|
||
Attached is a spec for the "rename" function. User tabs on text button to open up keyboard and text input box (please reference P2P sharing app -> Share My Apps -> "rename device").
Thanks
Comment 3•10 years ago
|
||
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8588907 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
Just wanted to get some feedback about the layout and styles of the description section (that includes Rename button).
Attachment #8588907 -
Flags: a11y-review?(amlee)
Assignee | ||
Comment 5•10 years ago
|
||
Doug, Justin, perhaps you have a suggestion, the UI described for renaming is basically gaia-dialog (used in sharing app described in comment 2). Should I be recreating that UI without the gaia-component? Or should I be using it, granted it might need to be browserified?
Flags: needinfo?(jdarcangelo)
Flags: needinfo?(drs)
Comment 6•10 years ago
|
||
Amy, the Settings app can't actually use the new web components yet as the rest of Gaia doesn't have the infrastructure to support it. We've been able to use WC in the Lightsaber apps because they were built outside the Gaia tree.
Could you adjust your visuals to use the old shared Building Blocks? We may eventually port the rest of Gaia over to WC, but it's not on the immediate horizon.
Yura, I would go ahead and re-implement this using BB for now. It's unfortunate, but I don't see a better way, even if this is landing on gaia/lightsaber only.
Flags: needinfo?(drs) → needinfo?(amlee)
Reporter | ||
Comment 7•10 years ago
|
||
I'm going to defer the question to Wilson, since he may know if we've already used gaia-dialog anywhere in Gaia or not. If gaia-dialog is not ready to be brought into Gaia, then I would take Doug's suggestion and just re-build it using BB.
Flags: needinfo?(jdarcangelo) → needinfo?(wilsonpage)
Comment 8•10 years ago
|
||
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #6)
> Amy, the Settings app can't actually use the new web components yet as the
> rest of Gaia doesn't have the infrastructure to support it. We've been able
> to use WC in the Lightsaber apps because they were built outside the Gaia
> tree.
I don't understand what this means. Why can't the Settings app use web-components?
Flags: needinfo?(wilsonpage)
Updated•10 years ago
|
Flags: needinfo?(drs)
Comment 9•10 years ago
|
||
My understanding is that gaia-header can be included in Gaia by Browserifying it with its dependencies, including gaia-component. Doing this for every component that we import would be very burdensome. On the other hand, Justin told me that the Camera app can actually import dependencies using Bower, but that they then have to be checked into the tree.
Neither option seems ideal to me, so it seems to make more sense to just wait until Gaia has better support for pulling in dependencies. If you disagree or I'm misunderstanding something, I'd be happy to include WC, as it would make things a lot easier for everyone.
Flags: needinfo?(drs)
Comment 10•10 years ago
|
||
(In reply to Yura Zenevich [:yzen] from comment #4)
> Comment on attachment 8588907 [details] [review]
> [gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
>
> Just wanted to get some feedback about the layout and styles of the
> description section (that includes Rename button).
Had a 1-on-1 with Yura to review style. He's going to see if he's able to use web components.
Flags: needinfo?(amlee)
Updated•10 years ago
|
Attachment #8588907 -
Flags: a11y-review?(amlee) → a11y-review+
Assignee | ||
Updated•10 years ago
|
Attachment #8588907 -
Flags: review?(arthur.chen)
Assignee | ||
Comment 11•10 years ago
|
||
This is an additional workaround the delete dialog, but it uses delay to wait for the app to be installed. It is not a nice UX when the addon is actually being deleted. If only there was a way to pass/get some event attribute on uninstall/install that would indicate that it's an update.
Comment 12•10 years ago
|
||
Comment on attachment 8588907 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
Hi Yura, I am trying to merge the settings app back to the lightsaber branch, would you mind to come up a patch against master? It is also fine if you do that after the merge.
And we are not using gaia-dialog in settings app (for keeping the style consistency with other apps). To avoid the lightsaber branch deviate too much from the master, I would suggest not to include WC for this time being.
Attachment #8588907 -
Flags: review?(arthur.chen)
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Arthur Chen [:arthurcc] from comment #12)
> Comment on attachment 8588907 [details] [review]
> [gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
>
> Hi Yura, I am trying to merge the settings app back to the lightsaber
> branch, would you mind to come up a patch against master? It is also fine if
> you do that after the merge.
>
> And we are not using gaia-dialog in settings app (for keeping the style
> consistency with other apps). To avoid the lightsaber branch deviate too
> much from the master, I would suggest not to include WC for this time being.
Hi Arthur, I am using gaia-gialog as it was one of the visual requirements to be in sync and consistent with other lightsaber related apps like Sharing, etc (see Amy's comment #2). In case that's too much overhead, I'll switch to DialogService.prompt in settings.
Flags: needinfo?(arthur.chen)
Comment 14•10 years ago
|
||
Hi Yura, I incline to make the style consistency be kept within an app instead of a feature. Please use DialogService for this time being.
Flags: needinfo?(arthur.chen)
Updated•10 years ago
|
Whiteboard: [lightsaber] → [ignite]
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8588907 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
Hi Arthur, I updated the PR to use DialogServices instead, hopefully it looks good. Also I was talking to Doug about moving things into master from lightsaber, I'm fine moving addon-details stuff into master either after this lands on lightsaber along with other changes from the branch, but can do it as part of this bug too. Let me know. Thanks!
Attachment #8588907 -
Flags: review?(arthur.chen)
Comment 16•10 years ago
|
||
Comment on attachment 8588907 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
Thanks for the patch, Yura. In general, the patch looks good. Please check my comments in github. And I found that addon.export always fails with a DOMError on my device, is a special gecko required for this feature?
Attachment #8588907 -
Flags: review?(arthur.chen)
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Arthur Chen [:arthurcc] from comment #16)
> Comment on attachment 8588907 [details] [review]
> [gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
>
> Thanks for the patch, Yura. In general, the patch looks good. Please check
> my comments in github. And I found that addon.export always fails with a
> DOMError on my device, is a special gecko required for this feature?
Yes, it was necessary to update to latest gecko with dev flag on. Also it depends on which addon you are exporting, pre-installed ones won't be exported I would think.
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8588907 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
All comments addressed, hopefully looks good now.
Attachment #8588907 -
Flags: review?(arthur.chen)
Comment 19•10 years ago
|
||
Comment on attachment 8588907 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
I was wrong about the lazy load thing. I just realized that jszip still got merged with the asynchronous syntax. Please add a new setting 'jszip': 'empty:' in settings.build.jslike specifying jszip should be excluded in any merge process.
r=me with the comments addressed, thanks.
Attachment #8588907 -
Flags: review?(arthur.chen) → review+
Comment 20•10 years ago
|
||
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8596021 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:lightsaber
Carrying over Amy's and Arthur's r+'s
Attachment #8596021 -
Flags: ui-review+
Attachment #8596021 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Attachment #8588907 -
Attachment is obsolete: true
Assignee | ||
Comment 22•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 23•10 years ago
|
||
Assignee | ||
Comment 24•10 years ago
|
||
Comment on attachment 8596727 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:master
Same PR to master
Attachment #8596727 -
Flags: review?(ejchen)
Attachment #8596727 -
Flags: review?(arthur.chen)
Comment 25•10 years ago
|
||
Comment on attachment 8596727 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:master
The code looks good to me. There are only a few comments to be addressed before merging. Please check them in github, thanks!
As for the style part, I saw there are some visual refinements done in this patch and I would like those to be addressed at once in bug 1147092. Would you mind only including the change for adding the rename button?
Attachment #8596727 -
Flags: review?(arthur.chen) → review+
Comment 26•10 years ago
|
||
Comment on attachment 8596727 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:master
Sorry, I meant to cancel the review.
Attachment #8596727 -
Flags: review+
Assignee | ||
Comment 27•10 years ago
|
||
Comment on attachment 8596727 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:master
Addressed comments, removed all styling changes.
Attachment #8596727 -
Flags: review?(ejchen) → review?(arthur.chen)
Comment 28•10 years ago
|
||
Comment on attachment 8596727 [details] [review]
[gaia] yzen:bug-1144849 > mozilla-b2g:master
r=me with the last comments addressed, thank you!
Attachment #8596727 -
Flags: review?(arthur.chen) → review+
Updated•10 years ago
|
Whiteboard: [ignite] → [spark]
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Keywords: checkin-needed
Comment 29•10 years ago
|
||
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
Comment 30•10 years ago
|
||
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
Assignee | ||
Comment 31•10 years ago
|
||
Comment 32•10 years ago
|
||
The design of the "Rename" button is basically a call for truncation.
https://transvision.mozfr.org/string/?entity=browser/installer/override.properties:Rename&repo=aurora
Take a look for example at Russian. Does something like "Переименование" fit in there?
Comment 33•10 years ago
|
||
(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #32)
> The design of the "Rename" button is basically a call for truncation.
> https://transvision.mozfr.org/string/?entity=browser/installer/override.
> properties:Rename&repo=aurora
>
> Take a look for example at Russian. Does something like "Переименование" fit
> in there?
The initial discussion was that Spark would only be in English for the initial release and so we did not look at truncation for other languages. Steph can you confirm?
Flags: needinfo?(swilkes)
You need to log in
before you can comment on or make changes to this bug.
Description
•