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

Formatted text table columns HLD and component naming conventions #1224

Merged
merged 49 commits into from
May 11, 2023

Conversation

jattasNI
Copy link
Contributor

@jattasNI jattasNI commented May 3, 2023

Pull Request

🤨 Rationale

Start the HLD for #1011 and other column types that display data as text. This update focuses on shared base classes, client-specified custom colums, and possible elements that Nimble could define, not APIs for specific elements.

We previously brainstormed this in #1054 but I started a new PR for this more concrete proposal.

👩‍💻 Implementation

This includes the HLD and also some updates to component naming conventions which fell out of the brainstorming.

🧪 Testing

N/A

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@jattasNI jattasNI marked this pull request as ready for review May 3, 2023 21:42
@jattasNI jattasNI mentioned this pull request May 10, 2023
1 task
@jattasNI jattasNI requested a review from rajsite May 10, 2023 22:09
@jattasNI jattasNI changed the title Formatted text table columns HLD Formatted text table columns HLD and component naming conventions May 10, 2023
Co-authored-by: Milan Raj <[email protected]>
@jattasNI jattasNI enabled auto-merge (squash) May 11, 2023 01:55
@jattasNI jattasNI disabled auto-merge May 11, 2023 02:11
@jattasNI jattasNI merged commit 394c07f into main May 11, 2023
@jattasNI jattasNI deleted the users/jattas/formatted-text-column-hld branch May 11, 2023 02:46
rajsite added a commit that referenced this pull request May 12, 2023
…1239)

# Pull Request

## 🤨 Rationale

As part of the vision described in #1224 in service of #1011 and other
column types, we will want to share rendering logic across columns that
display their data as text. This could eventually include:
 - the existing text column
 - the mapping column discussed in #1220 
 - columns that display numbers, dates, elapsed time, and more
 - columns defined in applications with custom formatting logic

## 👩‍💻 Implementation

1. Extract abstract base classes, templates, and styles from the
existing text column for:
    - the column custom element
    - the cell view custom element
    - the group header custom element
These live in `table-column/text-base`.
2. Derive concrete instances and register custom elements of each of
these for the `text` column
3. Define and use some table field types to provide stricter typing some
generic parameters

## 🧪 Testing

Relying on existing tests. 

We have a future task to add docs/examples/tests for applications
deriving from these base classes.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <[email protected]>
jattasNI added a commit that referenced this pull request Jun 15, 2023
# Pull Request

## 🤨 Rationale

Need APIs for #1011 and #1014. Builds on #1224.

## 👩‍💻 Implementation

Updates to existing HLD. Philosophy is to rely on the browser's `Intl`
formatting APIs when possible and to provide modes with opinionanted
configuration and with custom configuration.


## 🧪 Testing

N/A

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [X] I have updated the project documentation to reflect my changes or
determined no changes are needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants