Skip to content
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

Selection after search #333

Merged
merged 21 commits into from
Sep 27, 2024
Merged

Selection after search #333

merged 21 commits into from
Sep 27, 2024

Conversation

MaxiLein
Copy link
Contributor

@MaxiLein MaxiLein commented Jul 11, 2024

Summary

  • Fixed: When the user selected a process, then searched and then selected an additional process or selected all visible processes (Checkbox in header-row), the state saving the selected processes was populated with undefined values.
  • Added Tests for Selection over search and table-page change

Details

  • Added utility Data-Class (ObjectSetArrayType): An Array of objects, that behaves like a set, identifying an object based on an arbitrary number of pre-determined keys
  • Instances of such an Array-Set allow for usage of all array methods except fill (contradicting a set), as well as the set methods add and has
  • Functionality of the dataclass is complete, however, the autocompletion does not work optimal currently (created helper function for correct return-type). When using an array methods on an ObjectSetArray, the returned element is an ObjectSetArray again (copy and in-place), however, the type is array (Todo)

This comment has been minimized.

This comment has been minimized.

/* Ensure that the search input is cleared */
const inputSearch = await page.locator('.ant-input-affix-wrapper');
await inputSearch.getByPlaceholder(/search/i).fill('');

Copy link
Contributor Author

@MaxiLein MaxiLein Jul 11, 2024

Choose a reason for hiding this comment

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

When testing with moving into a folder, a check for being in the root folder before deleting could be beneficial, however, then the deletion loop needs to keep track of the processes to delete (deleting a folder deletes everything within it, therefore all ids within it should be removed from processListPage.processDefinitionIds or processListPage.folderIds as well).

This comment has been minimized.

This comment has been minimized.

Co-authored-by: jjoderis <jjoderis@users.noreply.github.com>

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Comment on lines 143 to 152
return [...new ObjectSetArray([...oldSelection, ...changeRows], 'id')] as T[];
} else {
return [
...new ObjectSetArray(
oldSelection.filter(
({ id }) => !changeRows.some(({ id: rowId }) => rowId === id),
),
'id',
),
] as T[];
Copy link
Contributor

Choose a reason for hiding this comment

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

That seems very complex for this use-case. Maybe using the built-in Set can implement this behaviour. Something like this:

function getUniqueObjects(array, ids) {
  const uniqueSet = new Set();
  const uniqueArray = [];

  for (const obj of array) {
    const key = JSON.stringify(Object.fromEntries(ids.map(id => [id, obj[id]])));
    if (!uniqueSet.has(key)) {
      uniqueSet.add(key);
      uniqueArray.push(obj);
    }
  }

  return uniqueArray;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did just that, although I think that the implemented ObjectSetArray holds nice additional array like utilities, I agree, that it is overkill here.

'process-list',
'del',
() => {
console.log('Hello World', canDeleteSelected);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment has been minimized.

This comment has been minimized.

Copy link

CLOUDRUN ACTIONS

✅ Successfully created Preview Deployment.

https://pr-333---ms-server-staging-c4f6qdpj7q-ew.a.run.app

@OhKai OhKai merged commit 7c78d0d into main Sep 27, 2024
11 checks passed
@OhKai OhKai deleted the selection-after-search branch September 27, 2024 22:49
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