Update homepage with log records from DB + pagination #77

kevin-pierce · 2023-05-31T01:47:22Z

GH ticket link

Pagination Component on Homepage

Related ticket
Update Homepage table to render data from DB

Implementation description

Updated get_log_records service + associated endpoint to take in results_per_page as parameter, allowing for chunked request sizes
Created Pagination component as per functional description + Figma designs
Extracted code for log records table into common/LogRecordsTable.tsx component
Lifted logic for SearchAndFilters into pages/LogRecords
Narrowed typing of common API client for log records

Functional Testing

Perform testing steps as outlined in Log Records Seeding Script #74 (this will setup enough sample data locally to test the remainder of this PR)
Verify that the total number of entries is correctly displayed under the left hand side of the table
Verify that the total number of pages corresponds to the (total number of entries / entries per page) + 1
Verify that the number of displayed records adheres to the currently selected records per page option
Verify that upon changing the records per page, we are sent back to page 1
Verify that the displayed records change upon changing pages
Verify that we can manually type in a page to navigate to in the page number text field
Verify that the "next" and "previous" page buttons are disabled when you're on the first / last page respectively
Verify that the container holding the log record entries is scrollable

What should reviewers focus on?

Structuring of the pages/LogRecords ; the design choice of lifting SearchAndFilters was done due to weird rendering issues, but we'd still like to get a second pair of 👀 on that
Should we be always resetting to Page 1 upon changing the results per page parameter?

Checklist

My PR name is descriptive and in imperative tense
My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
I have run the appropriate linter(s)
I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR
If I have made API changes, I have updated the REST API Docs
IF I have made changes to the db/models, I have updated the Data Models Page
I have updated other Docs as needed

…vs items dispaly

Safewaan

awesome stuff 💯 you guys popped off with this and got it done quickly, but there are a few front-end issues that should be fixed so #try-again #skill-issue #be-happy

frontend/src/components/common/LogRecordsTable.tsx

Safewaan · 2023-06-05T04:02:02Z

frontend/src/components/common/LogRecordsTable.tsx

+              return (
+                <Tr key={record.logId} style={{ verticalAlign: "middle" }}>
+                  <Td width="5%">
+                    {`${dateObj.getMonth()} ${dateObj.getDate()} ${dateObj.getFullYear()}`}


Would it be possible to show an abbreviated form for the month instead of the number? For example, instead of "9 2 2023", we would see Sept 9th, 2023.

Resolved ; used the following const array for the months. Let me know if you want any of these to be changed:

const MONTHS = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']

Following up on this ; the year is not included in the designs ; this was accidentally included by us. On that note though, is the year something we want to include?

yeah, might be confusing if you have two logs that are both jan 18th (but one is 2023, the other 2024)

the months are correct, but the year isn't showing:

frontend/src/components/common/LogRecordsTable.tsx

frontend/src/components/common/Pagination.tsx

frontend/src/components/common/LogRecordsTable.tsx

frontend/src/components/common/Pagination.tsx

frontend/src/components/pages/LogRecords.tsx

Safewaan · 2023-06-05T04:45:23Z

frontend/src/index.css

@@ -22,7 +22,7 @@ code {
  text-align: center;
  color: #2d3748;
  width: 75%;
-  padding-top: 10%;
+  padding-top: 2%;


this probably needs to get updated in #76 when that's merged

Safewaan · 2023-06-05T04:47:03Z

frontend/src/components/common/Pagination.tsx

+    <Box padding="12px 0px 33px">
+      <Flex justifyContent="space-between" alignItems="center">
+        <Box>
+          <Text color="#6D8788">{getNumRecordsStr()}</Text>


some manual styling will need to be updated when #76 is merged

kevin-pierce · 2023-06-06T02:32:03Z

TODOs:

Fix weird behaviour with page number text input
Add "Day Logs" header to table
Scrollbar should be reset to the top of the container when navigating to new page

Safewaan · 2023-06-06T13:51:07Z

@connor-bechthold @kevin-pierce can you folks also test the performance of the pagination? I noticed there was a slight delay when navigating to another page, but I don't have any issues with this unless the delay is notably longer with more log records. Please test to see what the delay is with like 10k log records and lmk 🐱

danielk1345

Not too many added comments since I think Safwaan covered most of them, but overall good work! I think the main changes and/or things to think about are the ones that Safwaan commented on (and the bug!)

danielk1345 · 2023-06-06T00:06:35Z

backend/app/services/interfaces/log_records_service.py

@@ -28,7 +28,7 @@ def add_record(self, log_record):
        pass

    @abstractmethod
-    def get_log_records(self, page_number, filters=None):
+    def get_log_records(self, page_number, results_per_page, filters=None):


If we have the default value for the results_per_page =1 in the implementations, maybe we should consider doing it the same for here

Good catch, will add 🙏

frontend/src/components/common/Pagination.tsx

Safewaan

just some issues with the input for the page number, let's sync with the designers to iron them out

Safewaan · 2023-06-14T14:19:57Z

frontend/src/components/common/Pagination.tsx

+            >
+              <Text>Page</Text>
+              <NumberInput
+                maxW="60px"


an issue with this is if there are 1000s of pages, the input box only shows 2 digits. This may be confusing to the user. I don't know what the width should be, but my (naïve) solution would be just to use a width that accommodates the largest possible integer.

Syncing with the designers on this would be a good first step.

Safewaan · 2023-06-14T14:23:30Z

frontend/src/components/common/Pagination.tsx

+              width="210px"
+            >
+              <Text>Page</Text>
+              <NumberInput


noticing if you keep typing numbers it'll go to infinity then back to 0 lol

maybe lets cap the input to the max number of pages (meaning if there are 400 pages and I'm at 300, I add another digit, it'll just go to 400 and won't let me add new numbers)

Safewaan · 2023-06-14T14:27:29Z

frontend/src/components/common/Pagination.tsx

+      newUserPageNum >= 1 &&
+      newUserPageNum <= numPages
+    ) {
+      getLogRecords(newUserPageNum);


one issue I have with getting the new logs whenever the input changes is that the pagination gets really slow (I'm testing with 100000 logs) and its hard to tell when you're on the right page. I think it makes more sense to get the log records once the input is actually submitted.

For example, if I start on page 1, and want to go to page 300, it'll fetch the logs for page 3, then page 30, then finally page 300. It should only retrieve the page 300 log records at the end, either by hitting enter on the input or clicking out of it.

A lot of this headache is coming from the designs 😖 maybe we can approach the UI for this differently

Safewaan · 2023-06-14T14:37:42Z

frontend/src/index.css

@@ -14,6 +14,15 @@ code {
    monospace;
 }

+.records-hero {


#76 is merged, so merge main and move these changes to the theme dir

Safewaan

lgtm! there's a few thing we need to improve (dealing with large page numbers, when to call the API, etc) but I'm happy with merging it in :D

Awesome work guys 🚀 now write 300 jest tests on pagination.

danielk1345

LGTM! There's a few small things with the linter and whatnot but it looks good to me!

Connor Bechthold added 2 commits May 27, 2023 00:11

base pagination setup and functionality

55accdb

styling and refactoring

514e5d3

kevin-pierce marked this pull request as ready for review June 1, 2023 04:53

kevin-pierce requested a review from Safewaan June 1, 2023 04:54

Connor Bechthold and others added 4 commits June 1, 2023 00:54

base pagination setup and functionality

7165ce6

styling and refactoring

a2c1aac

Updated items string to take into account number of records for item …

f2aed6b

…vs items dispaly

Linter on Pagination.tsx file

39233c8

kevin-pierce force-pushed the kevin/connor/pagination-component-on-homepage branch from 8c089a2 to 39233c8 Compare June 1, 2023 04:54

Safewaan requested a review from danielk1345 June 1, 2023 16:41

kevin-pierce added the front-end involves front-end work label Jun 3, 2023

Safewaan force-pushed the main branch from 3fdc012 to 54b10de Compare June 4, 2023 00:19

Safewaan mentioned this pull request Jun 4, 2023

Create Log Styling #82

Closed

2 tasks

Safewaan force-pushed the kevin/connor/pagination-component-on-homepage branch from ccb0e70 to 39233c8 Compare June 5, 2023 04:23

Safewaan requested changes Jun 5, 2023

View reviewed changes

Safewaan and others added 3 commits June 5, 2023 21:04

Merge branch 'main' into kevin/connor/pagination-component-on-homepage

89b8c7c

Resolved date formatting, added new date formatted util fn

abe8574

Removed comment in log_records_service

dab31b7

Resolved staff name

198e773

owen-sellner mentioned this pull request Jun 7, 2023

Design system #76

Merged

7 tasks

Connor Bechthold added 3 commits June 7, 2023 21:40

merge conflicts

0c9b866

address page number textbox comments

387d21c

add ref for table element to reset scroll on filter/page change

dcc2c0a

danielk1345 requested changes Jun 8, 2023

View reviewed changes

Connor Bechthold added 4 commits June 9, 2023 23:09

fix bug with page num not changing on results per page change

acbbc16

Merge branch 'main' into kevin/connor/pagination-component-on-homepage

abd6a64

add default results per page to get log records abstract method

c39e905

fix page number bug when backend throws error

794905b

kevin-pierce added 3 commits June 11, 2023 16:15

Added Day Logs hero

f14cd07

Migrated LogRecords to HomePage component + structure

10e8b3a

Moved around components

769b3aa

Safewaan requested changes Jun 14, 2023

View reviewed changes

Connor Bechthold added 5 commits June 16, 2023 00:04

fix merge conflicts

1c3f07e

merge fixes and fix page number box bounds

88c52ad

fix edge case

e3dc5fd

styling changes

2a92823

more styling changes

d150c69

connor-bechthold requested review from danielk1345 and Safewaan June 17, 2023 05:45

Connor Bechthold added 3 commits June 17, 2023 01:52

add default color to log records home page

90f06f5

change div to box

b077f67

nitpick changes

7cb378e

Safewaan approved these changes Jun 19, 2023

View reviewed changes

danielk1345 approved these changes Jun 19, 2023

View reviewed changes

linter run

7e38195

connor-bechthold merged commit 2f334fd into main Jun 20, 2023

connor-bechthold deleted the kevin/connor/pagination-component-on-homepage branch June 20, 2023 03:56

Safewaan linked an issue Jun 20, 2023 that may be closed by this pull request

Update Homepage table to render data from DB #70

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update homepage with log records from DB + pagination #77

Update homepage with log records from DB + pagination #77

kevin-pierce commented May 31, 2023 •

edited

Loading

Safewaan left a comment

Safewaan Jun 5, 2023

kevin-pierce Jun 6, 2023

kevin-pierce Jun 6, 2023

Safewaan Jun 6, 2023

kevin-pierce Jun 6, 2023

Safewaan Jun 8, 2023

Safewaan Jun 5, 2023

Safewaan Jun 5, 2023

kevin-pierce commented Jun 6, 2023

Safewaan commented Jun 6, 2023

danielk1345 left a comment

danielk1345 Jun 6, 2023

connor-bechthold Jun 10, 2023

Safewaan left a comment

Safewaan Jun 14, 2023

Safewaan Jun 14, 2023

Safewaan Jun 14, 2023

Safewaan Jun 14, 2023

Safewaan Jun 14, 2023

Safewaan left a comment

danielk1345 left a comment •

edited

Loading

Update homepage with log records from DB + pagination #77

Update homepage with log records from DB + pagination #77

Conversation

kevin-pierce commented May 31, 2023 • edited Loading

GH ticket link

Implementation description

Functional Testing

What should reviewers focus on?

Checklist

Safewaan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevin-pierce commented Jun 6, 2023

Safewaan commented Jun 6, 2023

danielk1345 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Safewaan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Safewaan left a comment

Choose a reason for hiding this comment

danielk1345 left a comment • edited Loading

Choose a reason for hiding this comment

kevin-pierce commented May 31, 2023 •

edited

Loading

danielk1345 left a comment •

edited

Loading