feat: introduce database content layout #713

dcshzj · 2024-09-30T09:57:23Z

Problem

Closes ISOM-1592.

Solution

Breaking Changes

Yes - this PR contains breaking changes
No - this PR is backwards compatible

Features:

Introduce a new SearchableTable component that creates a table that comes with a search filter and pagination controls.
Introduce a new database layout which locks the SearchableTable component at the bottom of the page.

Screenshots

vercel · 2024-09-30T09:57:27Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
isomer-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 11, 2024 3:15am

linear · 2024-09-30T10:02:15Z

ISOM-1592 Create a SearchableTable component

karrui

initial review.

should the searchtable also be useable on other layouts (i.e. is database layout necessary)?

packages/components/src/templates/next/components/internal/SearchableTable/SearchableTable.tsx

karrui · 2024-10-01T03:16:14Z

packages/components/src/templates/next/components/internal/SearchableTable/constants.ts

+// This is the maximum number of columns that the table can have
+// 10 was decided because at 1240px, each column will only have 124px which is
+// the minimum width for a column to be readable
+export const MAX_NUMBER_OF_COLUMNS = 10


in mobile will still look weird right haha. Might as well set min-width on each cell. Can always enhance it (and our other tables?) with something like https://www.npmjs.com/package/react-table-column-resizer (or write our own/vendorize) to allow users to resize columns.

Hmm this one to discuss with @sehyunidaaa. I think letting users resize the columns will not happen, but whether to use min-width I need to check.

If we set min-width (e.g., 124px) on the cols, will a table with 10 cols overflow and hori scroll on w=1025px (desktop)?

I think with the borders likely will overflow, so maybe we can use a smaller number such that the min-width of all columns + borders is <= 1024px?

Hmm I don't think overflow is necessarily bad when you have 10 (!!) columns, because at that point your data table is so big, that it's more important for us to guarantee that the column texts are readable than trying to avoid overflow. I'd rather the table be scrollable than we try to squeeze 10 columns into {1024px - margins} which might break words at awkward places.

I guess no issue in setting a min-width on columns, but should we be setting a max-width on the entire table instead? So we let the columns resize depending on content but prevent the scroll being ridiculously long, like 2* of the screen width

packages/components/src/templates/next/components/internal/SearchableTable/getFilteredItems.ts

datadog-opengovsg · 2024-10-03T05:12:23Z

Datadog Report

Branch report: feat/introduce-database-layout
Commit report: c5070a9
Test service: isomer-studio

✅ 0 Failed, 8 Passed, 1 Skipped, 7.98s Total Time

sehyunidaaa · 2024-10-08T05:27:33Z

@dcshzj sorry to bother you again! just fyi got some small items pending on chromatic, after you push again please let me know i'll approve chromatic

seaerchin

first pass review! some questoins on my end but overall seems ok. what's the point of the sentinel =HYPERLINK(? are we going to use it internally for github stored pages?

also, do we need an equivalent on studio for the layotu here?

packages/components/src/templates/next/components/internal/SearchableTable/CellContent.tsx

seaerchin · 2024-10-09T19:11:54Z

.../components/src/templates/next/components/internal/SearchableTable/SearchableTableClient.tsx

+
+ const maxNoOfColumns = Math.min(
+ headers.length,
+ ...items.map((row) => row.row.length),


what happens if we have a case where the header length is shorter than the max row length? do we have to handle overflow? (i never check on local)

This checks every row and finds the minimum number of columns. So if we have header row with 6 columns, 1st row with 6 columns and 2nd row with 3 columns, the max we should display is 3.

Overflow is handled automatically via scroll.

ah ok, so to double check what happens here - we only display 3 and there's a scrollbar allowing us to horizontally scroll and see the other 6? ok with this, i wanted to double check that we're not just removing the content

.../components/src/templates/next/components/internal/SearchableTable/SearchableTableClient.tsx

seaerchin · 2024-10-09T19:19:30Z

packages/components/src/templates/next/components/internal/SearchableTable/getFilteredItems.ts

+ searchValue,
+}: GetFilteredItemsParams) =>
+ items
+ .filter((item) => item.key.includes(searchValue.toLowerCase()))


if we want a case insensitive search, might as well lowercase the key!

Hmm the key is lowercased in the server component so the client does lesser stuff: https://github.com/opengovsg/isomer/pull/713/files#diff-5c209342b9152356c25bfdfc004d41eeb44c795172426b1bc17569d3b0e31bdb

not very good to assume here, we should just lowercase if it's cheap to do so. i think the runtime here won't be anyting significant

Hmm this is done as part of a performance improvement suggested here: #713 (comment)

I'm more inclined to reduce the work that the client does, especially since our users may have devices that are extremely slow.

ok! can add a comment on the studio side then, so people are aware of this!

Eh hmm there's no studio equivalent here, so the input is fine but on build time the key is generated with the lowercase.

packages/components/src/utils/getTransformedPageContent.ts

dcshzj · 2024-10-10T02:20:14Z

first pass review! some questoins on my end but overall seems ok. what's the point of the sentinel =HYPERLINK(? are we going to use it internally for github stored pages?

This is to support links. The intent is for people to create their table in Excel, export as CSV then upload to Studio in the future.

also, do we need an equivalent on studio for the layotu here?

Nope, editing on Studio for this layout is deprioritised for now, cos quite complicated to implement.

seaerchin · 2024-10-10T19:16:31Z

the logic here doesn't quite work! this is because we parse the text after we cache it. because of this, a false search of hyperlink will cause the result involving the link to show up!

in order to circumvent this, we should probably parse first prior to doing the cache construction so that we get a repsentation of the textual content.

dcshzj · 2024-10-11T03:07:56Z

the logic here doesn't quite work! this is because we parse the text after we cache it. because of this, a false search of hyperlink will cause the result involving the link to show up!

Updated in 79a1f81.

dcshzj requested a review from a team as a code owner September 30, 2024 09:57

dcshzj temporarily deployed to staging September 30, 2024 09:57 — with GitHub Actions Inactive

dcshzj force-pushed the feat/introduce-database-layout branch from b7cbb00 to a833e51 Compare October 1, 2024 02:25

dcshzj temporarily deployed to staging October 1, 2024 02:26 — with GitHub Actions Inactive

vercel bot deployed to Preview October 1, 2024 02:30 View deployment

dcshzj force-pushed the feat/introduce-database-layout branch from a833e51 to cba8e0a Compare October 1, 2024 03:01

dcshzj temporarily deployed to staging October 1, 2024 03:01 — with GitHub Actions Inactive

vercel bot deployed to Preview October 1, 2024 03:05 View deployment

karrui reviewed Oct 1, 2024

View reviewed changes

dcshzj temporarily deployed to staging October 1, 2024 03:44 — with GitHub Actions Inactive

vercel bot deployed to Preview October 1, 2024 03:47 View deployment

dcshzj temporarily deployed to staging October 1, 2024 06:43 — with GitHub Actions Inactive

dcshzj requested a review from karrui October 1, 2024 06:45

vercel bot deployed to Preview October 1, 2024 06:47 View deployment

dcshzj temporarily deployed to staging October 1, 2024 07:23 — with GitHub Actions Inactive

vercel bot deployed to Preview October 1, 2024 07:27 View deployment

dcshzj temporarily deployed to staging October 2, 2024 06:29 — with GitHub Actions Inactive

vercel bot deployed to Preview October 2, 2024 06:33 View deployment

vercel bot deployed to Preview October 3, 2024 05:09 View deployment

dcshzj temporarily deployed to staging October 3, 2024 05:09 — with GitHub Actions Inactive

dcshzj added 5 commits October 8, 2024 13:38

feat: introduce database content layout

a30dd11

feat: add support for links

d8b110b

fix: improve performance of searching

5448f81

chore: add story for search with no results

9099c64

chore: apply design fixes

fe5cb0a

dcshzj force-pushed the feat/introduce-database-layout branch from 019344a to fe5cb0a Compare October 8, 2024 05:38

vercel bot deployed to Preview October 8, 2024 05:38 View deployment

dcshzj temporarily deployed to staging October 8, 2024 05:38 — with GitHub Actions Inactive

fix: update empty state title prose type

5ae6bb2

vercel bot deployed to Preview October 8, 2024 05:40 View deployment

dcshzj temporarily deployed to staging October 8, 2024 05:40 — with GitHub Actions Inactive

seaerchin reviewed Oct 9, 2024

View reviewed changes

fix: handle undefined title

0b847a2

vercel bot deployed to Preview October 10, 2024 02:18 View deployment

dcshzj temporarily deployed to staging October 10, 2024 02:18 — with GitHub Actions Inactive

chore: remove hyperlink excel function in search key

79a1f81

vercel bot deployed to Preview October 11, 2024 03:05 View deployment

dcshzj temporarily deployed to staging October 11, 2024 03:05 — with GitHub Actions Inactive

dcshzj requested review from seaerchin and sehyunidaaa October 11, 2024 03:08

seaerchin approved these changes Oct 11, 2024

View reviewed changes

dcshzj merged commit e0fd6bb into main Oct 11, 2024
18 of 19 checks passed

dcshzj deleted the feat/introduce-database-layout branch October 11, 2024 08:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce database content layout #713

feat: introduce database content layout #713

dcshzj commented Sep 30, 2024 •

edited

Loading

vercel bot commented Sep 30, 2024 •

edited

Loading

linear bot commented Sep 30, 2024

karrui left a comment

karrui Oct 1, 2024

dcshzj Oct 1, 2024

sehyunidaaa Oct 1, 2024

dcshzj Oct 2, 2024

sehyunidaaa Oct 2, 2024

datadog-opengovsg bot commented Oct 3, 2024 •

edited

Loading

sehyunidaaa commented Oct 8, 2024

seaerchin left a comment

seaerchin Oct 9, 2024 •

edited

Loading

dcshzj Oct 10, 2024

seaerchin Oct 10, 2024

seaerchin Oct 9, 2024

dcshzj Oct 10, 2024

seaerchin Oct 10, 2024

dcshzj Oct 11, 2024

seaerchin Oct 11, 2024

dcshzj Oct 11, 2024

dcshzj commented Oct 10, 2024

seaerchin commented Oct 10, 2024

dcshzj commented Oct 11, 2024

feat: introduce database content layout #713

feat: introduce database content layout #713

Conversation

dcshzj commented Sep 30, 2024 • edited Loading

Problem

Solution

Screenshots

vercel bot commented Sep 30, 2024 • edited Loading

linear bot commented Sep 30, 2024

karrui 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

datadog-opengovsg bot commented Oct 3, 2024 • edited Loading

Datadog Report

sehyunidaaa commented Oct 8, 2024

seaerchin left a comment

Choose a reason for hiding this comment

seaerchin Oct 9, 2024 • edited Loading

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

dcshzj commented Oct 10, 2024

seaerchin commented Oct 10, 2024

dcshzj commented Oct 11, 2024

dcshzj commented Sep 30, 2024 •

edited

Loading

vercel bot commented Sep 30, 2024 •

edited

Loading

datadog-opengovsg bot commented Oct 3, 2024 •

edited

Loading

seaerchin Oct 9, 2024 •

edited

Loading