Skip to content

Update Slice Uids assigning#18

Open
shafeeqz wants to merge 3 commits intomicrosoft:mainfrom
shafeeqz:dev/shafeeqazzam/fix_slice_uids
Open

Update Slice Uids assigning#18
shafeeqz wants to merge 3 commits intomicrosoft:mainfrom
shafeeqz:dev/shafeeqazzam/fix_slice_uids

Conversation

@shafeeqz
Copy link
Contributor

@shafeeqz shafeeqz commented Dec 12, 2023

This changes provide support for slices with the same name if they differ in the selector field (not in the altConstantSelector).
If the slices with the same name have different selectors we keep the uid as is, else we change it.
We recommend using different selectors in the selector field for slices with the same name.

}

private buildFormattingSlices({slices, objectName, sliceNames, formattingSlices }: IBuildFormattingSlicesParams) {
private buildFormattingSlices({ slices, objectName, sliceNames, selectorsMap, formattingSlices }: IBuildFormattingSlicesParams) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create selector map here inside the method, You dont need to pass it from another method as a parameter

// so the correct thing to do is to provide a unique selector for each datapoint if needed.
// We are keeping sliceNames for backward compatibility.
// This map helps us to check if we need to change the uid
const selectorsMap: Record<string, { [selector: string]: boolean }> = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this map can be <string, [selector:string]>, In other wrods map of string which is the key to list of strings where its the selectors values list
You can add entry for slice name and its selectors, if a specific selector exist in the list it means it exist, no need for boolean

// Solution => Save slice names to modify each slice uid to be unique by adding counter value to the new slice uid
const sliceNames: { [name: string]: number } = {};

// creating slices using same uid as described above is ok at this stage, powerbi identify the specific slice by the selector.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please dont add and wrong ways, Since people will not know what was the main problem to create this.
So add description that we are supporting slices with same, And if selector (specify its not altConstantSelector) is different we keep the same uid for the slice, if not we have to change the uid (or else it wont work)
and add that the way when the slice name is similar and selector is different is the recommended way


if (formattingSlice) {
// Modify formatting slice uid if needed
const sliceName = slice.name;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you use this variable below?

}
} else {
sliceNames[slice.name]++;
const selectorExistsInMap = sliceSelector ? selectorsMap[slice.name][JSON.stringify(sliceSelector)] : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to replace undefined with false

selectorsMap[sliceName] = {};
sliceNames[slice.name] = 0;
if (sliceSelector) {
selectorsMap[slice.name][JSON.stringify(sliceSelector)] = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a variable for JSON.stringify(sliceSelector) as it is used in three places

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants