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

Migrated UserTable to KTable (Without Sorting) #13028

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

BabyElias
Copy link
Contributor

Summary

Migrated UserTable (as seen in UserPage.vue) to KTable.
This is with reference to #12705 which involves migrating Facility>Users table from the current UserTable implementation to KTable. Sorting functionality will be introduced in the next PR; this one focuses on checking any regressions that might have been made while migrating the Facility>Users table to a KTable-based implementation.

References

#12705

Reviewer guidance

  • Navigate to Facility>Users.
  • This page currently has both the original table (the top table) and the new KTable implementation (the bottom table). Once it's confirmed that KTable behaves similarly in all expected ways as the original table - the previous implementation will be removed.

@github-actions github-actions bot added APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) DEV: frontend labels Jan 24, 2025
@MisRob MisRob self-requested a review January 27, 2025 04:47
@MisRob MisRob self-assigned this Jan 27, 2025
@MisRob MisRob added the TODO: needs review Waiting for review label Jan 27, 2025
@MisRob MisRob requested a review from ozer550 January 28, 2025 18:41
@MisRob MisRob requested a review from radinamatic January 28, 2025 18:42
@MisRob
Copy link
Member

MisRob commented Jan 28, 2025

@radinamatic here the main focus of QA would be that there are no regressions in the KTable that is replacing the old table - e.g. that pagination still works, and any other table features that were here before. This work doesn't introduce any bugfixes or new features of KTable itself.

@ozer550
Copy link
Member

ozer550 commented Jan 30, 2025

I see a small difference for the superadmin row.

Original:
image
KTabel:
image

I'm not quite sure how relevant is this.

Also the table font size and style seems a bit diff. Altho I understand currently we might be just testing for regressions and not exactly replecating the old table?

@radinamatic
Copy link
Member

radinamatic commented Jan 30, 2025

It was a bit confusing that filtering and pagination features were affecting both tables... 😂

However all seems to be working without regressions, old ones and new KTable features, good to go! 👏🏽

2025-01-30_19-32-06

New font size noted by @ozer550 looks good to me 👍🏽 , and the role chip appears below probably because of the same column sizes (which can be changed, iirc).

Copy link
Member

@radinamatic radinamatic left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @BabyElias! 👍🏽 💯 :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) DEV: frontend TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants