Closed
Bug 972503
Opened 11 years ago
Closed 11 years ago
Allow home panel views to specify an initial filter
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(firefox30 fixed, firefox31 fixed)
RESOLVED
FIXED
Firefox 31
People
(Reporter: jdover, Assigned: jdover)
References
Details
Attachments
(1 file, 2 obsolete files)
7.21 KB,
patch
|
jdover
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Once filter navigation is finished in bug 942295, we should allow views within a panel specify an initial filter. This allows multiple views and/or panels to pull from the same dataset.
{ type: Home.panels.View.LIST,
dataset: "somedatasetid",
initialFilter: "somefilter" }
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → jdover
Assignee | ||
Comment 1•11 years ago
|
||
I added an optional 'rootFilter' key to the view config that can be used to specify the top level filter that should be used for the retrieving the initial data from the dataset.
Attachment #8396766 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Comment 2•11 years ago
|
||
Comment on attachment 8396766 [details] [diff] [review]
patch
Review of attachment 8396766 [details] [diff] [review]:
-----------------------------------------------------------------
Looks great but I think using 'rootFilter' kinda implies hierarchy navigation which is not necessarily true i.e. the add-on dev might just want to reuse same dataset across different views in the layout by just varying the initial filter. So, let's use something simpler. Rename rootFilter to simply 'filter'. This means:
In the json: rootFilter -> filter
In HomeConfig API: ViewConfig.getRootFilter() -> ViewConfig.getFilter()
And so forth. I originally considered going with 'initialFilter' instead but I think it's implied that a filter defined in ViewConfig is the initial filter anyway. No need to be too explicit about it.
Attachment #8396766 -
Flags: review?(lucasr.at.mozilla) → feedback+
Assignee | ||
Comment 3•11 years ago
|
||
rootFilter -> filter
Attachment #8396766 -
Attachment is obsolete: true
Attachment #8397268 -
Flags: review?(lucasr.at.mozilla)
Comment 4•11 years ago
|
||
Comment on attachment 8397268 [details] [diff] [review]
patch
Review of attachment 8397268 [details] [diff] [review]:
-----------------------------------------------------------------
Great. Please post an updated patch.
::: mobile/android/base/home/FramePanelLayout.java
@@ +38,5 @@
> Log.d(LOGTAG, "Loading");
>
> if (mChildView instanceof DatasetBacked) {
> + DatasetRequest request = new DatasetRequest(mChildConfig.getDatasetId(),
> + new FilterDetail(mChildConfig.getFilter(), null));
nit: maybe do:
final FilterDetail filter = new FilterDetail(mChildConfig.getFilter(), null);
final DatasetRequest request = new DatasetRequest(mChildConfig.getDatasetId(), filter);
::: mobile/android/base/home/HomeConfig.java
@@ +627,5 @@
> validate();
> }
>
> public ViewConfig(ViewType type, String datasetId, ItemType itemType,
> + ItemHandler itemHandler, String backImageUrl, String Filter) {
Filter -> filter
Attachment #8397268 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8397268 -
Attachment is obsolete: true
Attachment #8398193 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 6•11 years ago
|
||
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
Comment 8•11 years ago
|
||
Comment on attachment 8398193 [details] [diff] [review]
patch
Note to release managers: I'm requesting uplift for a series of Firefox Hub bugs. The main fixes we need are in bugs at the end of the series, but trying to rebase those patches proved difficult and risky, so I think we should just uplift the dependecies.
Note to sheriffs: I have a local patch series that I can land on aurora when these bugs all get approval.
[Approval Request Comment]
Bug caused by (feature/regressing bug #): dependency for initial Firefox Hub release (promoted feed add-ons in fx30)
User impact if declined: panels won't update when data changes
Testing completed (on m-c, etc.): baked on m-c
Risk to taking this patch (and alternatives if risky): only affects dynamic panels, need an add-on to trigger this
String or IDL/UUID changes made by this patch: none
Attachment #8398193 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #8398193 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 9•11 years ago
|
||
status-firefox30:
--- → fixed
status-firefox31:
--- → fixed
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•