Frontend support for configurable record summaries #3987

seancolsen · 2024-10-25T19:53:52Z

⚠️ Do not merge until #3997 is merged.

Fixes: #3890

Demo

2024-10-31_15-49-20.mp4

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the develop branch of the repository
My commit messages follow best practices.
My code follows the established code style of the repository.
I added tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

We actually only need this in ONE place: `identifyAbstractTypeForDbType`. Now that it's a static value we can stop passing it around and importing it all over the codebase!

seancolsen · 2024-10-31T20:08:17Z

Ok this is working pretty well now, and I'd like to start review.

@ghislaineguerin I've assigned you because I'd like you to be the first to review. I want to make sure we're agreed on the UX before moving on to any code-level review. Watch the demo video or play around with it locally. I'm happy to hop on a call to chat more if you like. Once you feel good about it, then I'll open it up to Pavish for code review.

@kgodey I you might like to watch the demo video too in case you have any critique of the new UX here.

ghislaineguerin

@seancolsen Based on the video this looks great. I think the proposed UX works.

seancolsen · 2024-11-12T01:45:14Z

Thanks @ghislaineguerin.

@pavish I'd like you to review this now. Keep in mind it's stacked on #3997 which you're also assigned to review.

pavish · 2024-11-12T18:22:27Z

mathesar_ui/src/systems/data-explorer/utils.ts

+                      // TODO_BETA: Ask Pavish.
+                      //
+                      // `Column['type_options']` was previously typed loosely
+                      // as `Record<string, unknown> | null`. Now it's more
+                      // strict and it doesn't have a `type` property.
+                      //
+                      // type:
+                      //   updatedColumnsMetaData.get(aggregation.inputAlias)
+                      //     ?.column.type ?? 'unknown',
+                    }


@seancolsen We discussed this on a 1:1 before. I think we missed making the code change.

type here is now changed to item_type. This applies for _array types which require the type of items they hold.

pavish

@seancolsen This looks great! Nice work!

I haven't reviewed the code in detail, I mostly scanned for lines that stood out and they all seem fine.

seancolsen added this to the v0.2.0 (beta release) milestone Oct 25, 2024

Base automatically changed from record_summaries to develop October 28, 2024 07:24

seancolsen force-pushed the record_summaries_ui branch from dc4eb8e to 77dfb27 Compare October 30, 2024 02:51

seancolsen added 16 commits October 30, 2024 14:33

Adapt front end to new record summary types

57a6a1a

Eliminate abstractTypesMap parameters

07e90d2

We actually only need this in ONE place: `identifyAbstractTypeForDbType`. Now that it's a static value we can stop passing it around and importing it all over the codebase!

Create Fieldset component

52471da

Scaffold record summary editing UI

30d1f03

Improve appearance of field selectors

7df5251

Implement record summary previews

f254994

Dispatch change event from RadioGroup

43418ad

Handle template customization radio field

b5f606b

Fix preview with null template

212f682

Handle template updates

f9369dd

Adjust copy

8b6dd7c

Don't re-render template parts when sorting parts

89baaed

Handle saving record summary template

347148c

Adjust visual spacing

8897301

Use TemplateConfig data structure

e6c9798

Enable record summary config for FK columns

56e5a7c

seancolsen force-pushed the record_summaries_ui branch from 77dfb27 to 56e5a7c Compare October 30, 2024 18:37

seancolsen changed the base branch from develop to persist_inspector_section_visibility October 30, 2024 18:37

seancolsen mentioned this pull request Oct 30, 2024

Persist visibility of table inspector sections #3997

Merged

7 tasks

seancolsen added 8 commits October 30, 2024 20:26

Fix bug when toggling template customization

f85c2d2

Improving handling of empty record summaries

e3c2d8f

Add form validation to prevent static templates

b11ceda

Improve UI state for empty template

1afaa73

Abandon some low-priority TODO items

af9c097

Improve help content

e560ac0

Record summary i18n

ffc597e

Reload records after updating linked record summary

c7366cd

seancolsen added 3 commits October 31, 2024 13:29

Gracefully handle tables without primary keys

6af3662

Fix loading behavior

93ba873

Gracefully handle deleted columns in template

c33b504

seancolsen marked this pull request as ready for review October 31, 2024 20:07

seancolsen assigned ghislaineguerin Oct 31, 2024

ghislaineguerin approved these changes Nov 11, 2024

View reviewed changes

seancolsen assigned pavish and unassigned ghislaineguerin Nov 12, 2024

Base automatically changed from persist_inspector_section_visibility to develop November 12, 2024 15:12

Merge branch 'develop' into record_summaries_ui

f317e04

pavish reviewed Nov 12, 2024

View reviewed changes

pavish approved these changes Nov 12, 2024

View reviewed changes

pavish added this pull request to the merge queue Nov 12, 2024

Merged via the queue into develop with commit 20b9906 Nov 12, 2024
47 checks passed

pavish deleted the record_summaries_ui branch November 12, 2024 18:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Frontend support for configurable record summaries #3987

Frontend support for configurable record summaries #3987

seancolsen commented Oct 25, 2024 •

edited

Loading

seancolsen commented Oct 31, 2024

ghislaineguerin left a comment

seancolsen commented Nov 12, 2024

pavish Nov 12, 2024 •

edited

Loading

pavish left a comment

Frontend support for configurable record summaries #3987

Frontend support for configurable record summaries #3987

Conversation

seancolsen commented Oct 25, 2024 • edited Loading

Demo

Checklist

Developer Certificate of Origin

seancolsen commented Oct 31, 2024

ghislaineguerin left a comment

Choose a reason for hiding this comment

seancolsen commented Nov 12, 2024

pavish Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

pavish left a comment

Choose a reason for hiding this comment

seancolsen commented Oct 25, 2024 •

edited

Loading

pavish Nov 12, 2024 •

edited

Loading