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.

Sign up for GitHub

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

Jump to bottom

Refactor table component to functional #4410

Merged
merged 1 commit into from Jan 27, 2024
Merged

Refactor table component to functional #4410

merged 1 commit into from Jan 27, 2024

Conversation

norbye
Copy link
Member

@norbye norbye commented Jan 25, 2024 •
edited

Description

Refactor Table component to functional and split it over multiple files

  • Change checkbox to radio button
    • The behaviour has always been corresponding to Radio buttons, so I have no idea why they were checkboxes in the first place.
    • I have a PR out that was intended to change the behaviour to match checkboxes (Enable selecting multiple filter items #4257) - but on second though I'd rather refactor first (this PR) - then touch up the code in the other PR to support checkbox/radiobutton depending on the table and column, as not all filters are suited to be checkboxes
  • Prevent autoscroll on open searchbar
    • If you go to prod/staging and scroll a little bit down before you click the search-icon on tables, you will be scrolled to the top of the page. This was annoying. This is gone.
  • Members list seemed to have a bug in dev where the group be accessed before being loaded - ? to the rescue

Result

If you've made visual changes, please include before and after images, preferably with a description. Make sure they do not contain any real user information.

Before

Skjermopptak.2024-01-25.kl.15.24.50.mov

After

Skjermopptak.2024-01-25.kl.15.20.15.mov

Testing

  • I have thoroughly tested my changes.

Tested a on a few different tables with local backend and staging

Resolves ABA-763

@norbye norbye added review-needed Pull requests that need review bug-fix Pull requests that fix a bug chore Pull requests that does something "boring", yet important, e.g. cleaning up code labels Jan 25, 2024
@linear Linear
Copy link

linear bot commented Jan 25, 2024

ABA-763 Refactor table component

@norbye norbye requested a review from a team January 25, 2024 02:39
@norbye
Refactor table component to functional
3a6e831 
Change checkbox to radio
Prevent autoscroll on open searchbar
falbru
falbru approved these changes Jan 25, 2024
View reviewed changes
Copy link
Contributor

@falbru falbru left a comment

Choose a reason for hiding this comment

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

Nice!

itsisak
itsisak approved these changes Jan 25, 2024
View reviewed changes
Copy link
Contributor

@itsisak itsisak left a comment

Choose a reason for hiding this comment

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

LGMT!

ivarnakken
ivarnakken approved these changes Jan 26, 2024
View reviewed changes
Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

I suppose some types here and there could be better, but this is nevertheless a big improvement! Thanks for refactoring this nasty component! <3

app/components/Table/index.tsx Show resolved Hide resolved
app/components/Table/index.tsx Show resolved Hide resolved
app/components/Table/BodyCell.tsx Show resolved Hide resolved
@ivarnakken ivarnakken added types Pull requests that improve or fix types approved Pull requests that have been approved technical-debt Pull requests that reduces technical debt labels Jan 26, 2024
@norbye
Copy link
Member Author

norbye commented Jan 26, 2024

I suppose some types here and there could be better, but this is nevertheless a big improvement! Thanks for refactoring this nasty component! <3

Ah yeah - those anys where there from before and this PR felt big enough. I've fixed most of them in the next Table-PR that is coming with improved filtering - coming soon to a repo near you

I'm keeping your comments unresolved to make sure all of them are addressed in the next iteration(:

@norbye norbye merged commit 7117e5c into master Jan 27, 2024
4 checks passed
@norbye norbye deleted the refactor-table branch January 27, 2024 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itsisak itsisak itsisak approved these changes

@falbru falbru falbru approved these changes

@ivarnakken ivarnakken ivarnakken approved these changes

Assignees
No one assigned
Labels
approved Pull requests that have been approved bug-fix Pull requests that fix a bug chore Pull requests that does something "boring", yet important, e.g. cleaning up code review-needed Pull requests that need review technical-debt Pull requests that reduces technical debt types Pull requests that improve or fix types
Projects
None yet
Milestone
No milestone
4 participants
@norbye @itsisak @falbru @ivarnakken