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

Feature/filter component #313

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Feature/filter component #313

merged 1 commit into from
Mar 19, 2024

Conversation

seanes
Copy link
Collaborator

@seanes seanes commented Mar 16, 2024

Dynamisk filterkomponent (Filterbar) ansvarlig for visning og kontroll av filtre, som generer et sett med filtre for filtrering av data.

Hva er endret?

Dette er første iterasjon.

Dokumentasjon / Storybook / testdekning

Eksempler i Storybook.

  • Dokumentasjon er oppdatert eller ikke relevant / nødvendig.
  • Ny komponent har en eller flere stories i Storybook, eller så er ikke dette relevant.
  • Det er blitt lagt til nye tester / eksiterende tester er blitt utvidet, eller tester er ikke relevant.

Skjermbilder eller GIFs (valgfritt)

image

image

@seanes seanes requested a review from a team as a code owner March 16, 2024 08:32
@seanes seanes force-pushed the feature/filter_component branch 2 times, most recently from 056fbf4 to 20e54bb Compare March 18, 2024 08:13
@seanes seanes force-pushed the feature/filter_component branch from 50483b9 to 1e5913a Compare March 18, 2024 09:55
align-items: center;
justify-content: center;
background-color: rgb(243, 244, 246);
color: rgba(0,0,0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

flisespikk, men hvorfor ikke bare skrive "black" og spare å kjøre en funksjon? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

det skal byttes uansett, koster ingenting

<Button size="small" onClick={onBtnClick} disabled={disabled} variant="secondary" color="first">
<PlusIcon /> {t('filter_bar.add_filter')}
</Button>
{isOpen && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially refactor this into a <ButtonWithDropdown />-component? To extract the button, isOpen and dropdown functionality.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Vi må se hvor gjenbrukbart det kan bli på sikt, og behovene. Ser ingen verdi i å skille ut dette som egen komponent foreløpig.

* />
* ```
*/
export const FilterBar = ({ onFilterChange, fields, initialFilters = [] }: FilterBarProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could write a unit test for this component?

Copy link
Contributor

@TheKnarf TheKnarf left a comment

Choose a reason for hiding this comment

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

PR looks great, missing unit tests (but that could potentially be its own PR).

@seanes
Copy link
Collaborator Author

seanes commented Mar 19, 2024

Venter med enhetstester til denne er blitt litt mer moden og vi er blitt enige om på hvilket nivå vi skal legge oss.

@seanes seanes merged commit 76f846b into main Mar 19, 2024
8 checks passed
@seanes seanes deleted the feature/filter_component branch March 19, 2024 08:36
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