feat(panel): Panel grouping #9538
Conversation
| * open within a defined panel group. | ||
| */ | ||
| MdPanelService.prototype.newPanelGroup = function(groupName, config) { | ||
| if (!this._groups[groupName]) { |
There was a problem hiding this comment.
I think that it might be better to use hasOwnProperty here. This might cause undefined behaviour in the edge case if somebody names their group toString or something similar.
crisbeto
added
the
in progress
| method. | ||
| </p> | ||
| <p> | ||
| Adding panels to groups prevents the panel API from storing a ridiculous |
There was a problem hiding this comment.
reword - "rediculous amount of data" sounds too vague as does the added flexibility. This is your sales pitch for groups.
|
|
||
| var panel = $mdPanel.create(config); | ||
|
|
||
| panel.open(); |
There was a problem hiding this comment.
Please use the test helper methods for these:
openPanel(config); and closePanel().
There was a problem hiding this comment.
@ErinCoughlan Unless I am mistaken, this change will disrupt the expect tests that I am running in this test. I used this method because I needed to create a panel variable to then check the open panels array for afterward. Please let me know if I didn't write this test correctly.
There was a problem hiding this comment.
@bradrich Unless I'm really confused about something in that util, you get the panelRef back, so your tests still work. Well, you have to rename panel to panelRef...
openPanel(config);
expect(getPanels).toContain(panelRef);
|
A few minor comments left. Once resolved, LGTM. |
bradrich
force-pushed
the
feat/panel-grouping
branch
2 times, most recently
from
b3ffd49 to
bb6c152
Compare
|
@ErinCoughlan @crisbeto All changes and nits have been accounted for. Thanks for the solid review. |
bradrich
added
pr: merge ready
bradrich
force-pushed
the
feat/panel-grouping
branch
from
bb6c152 to
40771b2
Compare
ThomasBurleson
added
needs: presubmit
and removed
pr: merge ready
bradrich
force-pushed
the
feat/panel-grouping
branch
from
40771b2 to
e6cf876
Compare
jelbourn
added
needs: rebase
|
Passed presubmit, but needs rebase. |
bradrich
force-pushed
the
feat/panel-grouping
branch
from
e6cf876 to
ee7532c
Compare
|
@jelbourn Rebase complete, please continue. |
|
@devversion Are the TravisCI checks failing due to something that I can fix? I have rebased and there is still a failure... |
|
@bradrich Yeah that's not a CI issue. You have a syntax error in the spec file. |
|
@devversion Ah, found it. It was the callInterceptors function that didn't have its final |
bradrich
force-pushed
the
feat/panel-grouping
branch
from
ee7532c to
0835da8
Compare
|
@jelbourn There was a missing bracket that I discovered in the test files and removed. Please continue. |
Panels are now capable of being grouped together by any designation the user sees fit. Grouping allows the user to specify logic for a collection of panels. For instance, all popups in the header can be considered one group with a maximum of one popup open at a time. Dialogs within dialogs are another example of groups. This implementation prevents the panel from storing a ridiculous amount of data in memory or polluting the DOM with several panels. This helps performance and gives the user more flexibility about how to use panels. Fixes angular#8971
bradrich
force-pushed
the
feat/panel-grouping
branch
from
0835da8 to
eb6d8b8
Compare
ThomasBurleson
added
needs: presubmit
and removed
needs: rebase
Panels are now capable of being grouped together by any designation the user sees fit. Grouping allows the user to specify logic for a collection of panels. For instance, all popups in the header can be considered one group with a maximum of one popup open at a time. Dialogs within dialogs are another example of groups. This implementation prevents the panel from storing a ridiculous amount of data in memory or polluting the DOM with several panels. This helps performance and gives the user more flexibility about how to use panels. Fixes angular#8971
Panels are now capable of being grouped together by any designation the user sees fit. Grouping allows the user to specify logic for a collection of panels. For instance, all popups in the header can be considered one group with a maximum of one popup open at a time. Dialogs within dialogs are another example of groups. This implementation prevents the panel from storing a ridiculous amount of data in memory or polluting the DOM with several panels. This helps performance and gives the user more flexibility about how to use panels.
Fixes #8971
Ping @ErinCoughlan