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

[SALAD-23962] Add machine table dropdown menu #1270

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rners01
Copy link
Contributor

@rners01 rners01 commented Jan 30, 2025

image

@rners01 rners01 self-assigned this Jan 30, 2025
@rners01 rners01 requested a review from a team as a code owner January 30, 2025 03:03
Copy link

@rners01 rners01 force-pushed the add-machine-table-dropdown-menu branch from 7a41318 to a302d8d Compare January 30, 2025 10:00
packages/web-app/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
<FontAwesomeIcon icon={faList} />
<Dropdown
control={<FontAwesomeIcon size="xl" icon={faList} />}
options={[
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move options in separate constant

@@ -108,7 +109,28 @@ const _AllMachines = ({ classes }: Props) => {
const getTitles = () => {
return [
<div className={(classes.tableHeaderCell, classes.tableCellCentered)}>
<FontAwesomeIcon icon={faList} />
<Dropdown
Copy link
Contributor

Choose a reason for hiding this comment

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

It didn't select items. Will this functionality be added in the following PR?

@vitto-moz
Copy link
Contributor

vitto-moz commented Jan 30, 2025

Dropdown options are aligned left on design
image

@vitto-moz
Copy link
Contributor

vitto-moz commented Jan 30, 2025

  1. let's make the dropdown functionality connected to the table.
  2. looks like there is no need to keep table and dropdown state synced. On the other hand if we need it - let's make it work consistent
    image

@rners01 rners01 force-pushed the add-machine-table-dropdown-menu branch 2 times, most recently from 0b192bd to eb5e222 Compare January 31, 2025 11:19
Comment on lines +120 to +134
const beginningIndex = (currentPageNumber - 1) * itemsPerPageAmount
const endingIndex = currentPageNumber * itemsPerPageAmount
const machinesToSelect = generatedMockedMachines
.slice(beginningIndex, endingIndex)
.reduce((acc, machine) => ({ ...acc, [machine.id]: true }), {})
Copy link
Contributor

Choose a reason for hiding this comment

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

let's keep it completely aligned with usePagination() hook values.

Suggested change
const beginningIndex = (currentPageNumber - 1) * itemsPerPageAmount
const endingIndex = currentPageNumber * itemsPerPageAmount
const machinesToSelect = generatedMockedMachines
.slice(beginningIndex, endingIndex)
.reduce((acc, machine) => ({ ...acc, [machine.id]: true }), {})
const machinesToSelect = generatedMockedMachines
.slice(lowestItemNumberOnPage - 1, highestItemNumberOnPage)
.reduce((acc, machine) => ({ ...acc, [machine.id]: true }), {})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 140 to 152
const beginningIndex = (currentPageNumber - 1) * itemsPerPageAmount
const endingIndex = currentPageNumber * itemsPerPageAmount
const machinesToDeselect = generatedMockedMachines
.slice(beginningIndex, endingIndex)
.reduce((acc, machine) => ({ ...acc, [machine.id]: false }), {})
Copy link
Contributor

Choose a reason for hiding this comment

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

let's keep it completely aligned with usePagination() hook values.

Suggested change
const beginningIndex = (currentPageNumber - 1) * itemsPerPageAmount
const endingIndex = currentPageNumber * itemsPerPageAmount
const machinesToDeselect = generatedMockedMachines
.slice(beginningIndex, endingIndex)
.reduce((acc, machine) => ({ ...acc, [machine.id]: false }), {})
const machinesToDeselect = generatedMockedMachines
.slice(lowestItemNumberOnPage - 1, highestItemNumberOnPage)
.reduce((acc, machine) => ({ ...acc, [machine.id]: false }), {})

@rners01 rners01 force-pushed the add-machine-table-dropdown-menu branch from eb5e222 to 8bcbc3c Compare February 6, 2025 07:07
@rners01 rners01 requested review from Maks19 and vitto-moz February 6, 2025 14:15
@rners01 rners01 force-pushed the add-machine-table-dropdown-menu branch from 8bcbc3c to ad570be Compare February 7, 2025 05:35
customStyles?: DropdownStylesConfig
options?: DropdownOption[]
value?: string
isSearchable?: boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this functionality?

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