Skip to content

OBPIH-6513+OBPIH-6514 Indicator for items requiring attention + include file row…#4813

Merged
awalkowiak merged 2 commits intofeature/outbound-importfrom
OBPIH-6513
Aug 30, 2024
Merged

OBPIH-6513+OBPIH-6514 Indicator for items requiring attention + include file row…#4813
awalkowiak merged 2 commits intofeature/outbound-importfrom
OBPIH-6513

Conversation

@kchelstowski
Copy link
Collaborator

… number in the table

@github-actions github-actions bot added domain: frontend Changes or discussions relating to the frontend UI domain: l10n Changes or discussions relating to localization & Internationalization labels Aug 30, 2024
@kchelstowski kchelstowski changed the title OBPIH-6513 Indicator for items requiring attention + include file row… OBPIH-6513+OBPIH-6514 Indicator for items requiring attention + include file row… Aug 30, 2024
Comment on lines 4 to 10
const [isFiltered, setIsFiltered] = useState(false);

return {
setIsFiltered,
isFiltered,
};
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's worth moving to this hook more logic related to filtering? Generally speaking, having a hook with just one setState is not worth creating this hook 😄
I think you can move to this hook actions like:

setIsFiltered((prev) => !prev);

and call it toggleFiltering
or things like that:

isFiltered
          ? (itemsWithErrors ?? [])
          : (itemsInOrder ?? []

Copy link
Collaborator

Choose a reason for hiding this comment

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

Other than this set state hook, lgtm

@awalkowiak awalkowiak merged commit c5f0194 into feature/outbound-import Aug 30, 2024
@awalkowiak awalkowiak deleted the OBPIH-6513 branch August 30, 2024 12:57
awalkowiak pushed a commit that referenced this pull request Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: frontend Changes or discussions relating to the frontend UI domain: l10n Changes or discussions relating to localization & Internationalization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants