-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support group in widget item #118
Conversation
Preview is ready. |
src/utils/update-manager.ts
Outdated
itemIds = [id].concat(item.data.tabs.map((tab) => tab.id)); | ||
} | ||
if (isItemWithGroup(item)) { | ||
itemIds = [id].concat(item.data.group.map((tab) => tab.id)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tab -> group
src/shared/modules/helpers.ts
Outdated
config: Config; | ||
itemsStateAndParams: ItemsStateAndParams; | ||
} | ||
|
||
function getActualItems(items: ConfigItem[]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here u receive items ids - not items
maybe rename like getActualItemsIds
?
}).toEqual(addToQueue({id: DEFAULT_CONTROL_ID, config, itemsStateAndParams})); | ||
}); | ||
|
||
it('remove unused items from queue after adding new item', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can u add case where we have 1 items with 2 different groups in queue but one group id doesn't exist in the config
? And we add something to queue - it should delete this group id from queue
src/utils/update-manager.ts
Outdated
@@ -392,6 +531,7 @@ export class UpdateManager { | |||
const newTabId: string | undefined = stateAndParams.state?.tabId; | |||
const isTabSwitched = isItemWithTabs(initiatorItem) && Boolean(newTabId); | |||
const currentMeta = getItemsStateAndParamsMeta(itemsStateAndParams); | |||
const groupItemId = options?.groupItemId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we add options.groupItemsIds
instead of options.groupItemId
?
Maybe sometimes you don't need update all groups in queue or you want to control order in queue for update all groups.
* feat: support group in widget item * chore: add tests * fix: review issues
* feat: support group in widget item * chore: add tests * fix: review issues
* feat: support group in widget item * chore: add tests * fix: review issues
Support for plugins that include a group of items. Each of the items should affect the state as if it were an independent element, but still be part of the same widget
The changes include edits in the setItem, the formation of queues and a state, the addition of a method
addGroupToQueue
In
onStateAndParamsChange
there's need to putgroupItemsIds
in options for correct work