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.

Sign up for GitHub

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

Jump to bottom

Initial table implementation #840

Closed
9 tasks done
atmgrifter00 opened this issue Dec 1, 2022 · 6 comments · Fixed by #902
Closed
9 tasks done

Initial table implementation #840

atmgrifter00 opened this issue Dec 1, 2022 · 6 comments · Fixed by #902
Assignees
@mollykreis @atmgrifter00
Labels
new component Request for a new component
Milestone
Table Milestone 1

Comments

@atmgrifter00
Copy link
Contributor

atmgrifter00 commented Dec 1, 2022 •
edited by nate-ni
Loading

📌 User Story

This user story covers the creation of the nimble-table component, and any other components needed to render a set of data as outlined in the data API HLD (spec reference coming).

Accessibility

This basic table should have appropriate aria labels to meet accessibility standards
This version of the table will use the table role
https://w3c.github.io/aria-practices/examples/table/table.html
https://w3c.github.io/aria-practices/#table

Design

This table should implement the basic visual design:

Platform support

The basic table should have versions available for Angular and Blazor

Other functionality

If the data of the table updates, the table should update

  • This behavior allows existing workflows of filtering and searching to work.

📋 Tasks
Beta Give feedback
  1. Visual design for initial table implementation #883
    1 of 1
    UX-Deprecated
    RickA-NI

Related PRs
Beta Give feedback
  1. Basic TanStack integration with nimble-table #902
  2. Create Angular directive for nimble-table #914
  3. Create nimble-table-row and nimble-table-cell and use them in the table #921
  4. Table column header component #928
  5. Basic styling for the nimble-table #931
  6. Blazor nimble table #932
  7. Update XD link to table design #945
  8. Update table styles #969
@atmgrifter00 atmgrifter00 added this to the Table Alpha Release milestone Dec 1, 2022
@mollykreis mollykreis self-assigned this Dec 1, 2022
@nate-ni nate-ni added new component Request for a new component UX labels Dec 4, 2022
@nate-ni nate-ni moved this to 🏗 In progress in Nimble Design System Priorities Dec 5, 2022
@nate-ni nate-ni removed the UX label Dec 5, 2022
This was referenced Dec 6, 2022
Closed
Closed
@nate-ni nate-ni linked a pull request Dec 7, 2022 that will close this issue
Basic TanStack integration with nimble-table #902
Merged
1 task
@nate-ni nate-ni removed a link to a pull request Dec 7, 2022
Basic TanStack integration with nimble-table #902
Merged
1 task
@nate-ni nate-ni mentioned this issue Dec 7, 2022
Merged
1 task
mollykreis added a commit that referenced this issue Dec 9, 2022
@mollykreis @jattasNI @rajsite
Basic TanStack integration with nimble-table (#902)
a4ce35d 
# Pull Request

## 🤨 Rationale

This PR is part of the effort for #840.

This PR pulls in the TanStack dependency to nimble-components and
updates the `nimble-table` to render basic data by leveraging TanStack.

Things that are temporary in this PR:
- Determining the set of columns by looking at the keys in the data
- This will ultimately be determined by column definitions provided to
the table (likely slotted as children of the table)
- Turning the data into `string[][]` type for rendering
- This will ultimately use view templates provided by the column
definitions
- The entire template, including styling
- This will ultimately be updated to have additional `nimble-*` elements
that make up the table. Some examples may be `nimble-column-header`,
`nimble-row`, and `nimble-cell`.
- The template will ultimately need to be setting appropriately ARIA
attributes

## 👩‍💻 Implementation

- Add a dependency on the latest version of TanStack
- Use the TanStack APIs to create a table using the basic
`getCoreRowModel()` function. Currently the only relevant information
we're giving TanStack is the data, but as development continues on the
table, the same patterns can be used for other information, such as
sorting & grouping state.
- Create an interface named `TableData` that describes the required type
of `TData` passed to the table
- Provide a property on the `nimble-table` that allows setting data,
which is rendered in the table

## 🧪 Testing

- Manually tested via storybook
- Wrote unit tests, primarily focused around being able to update the
data in the table
- As much/all of the template is likely to change, I created a page
object for the table in hopes that it will help us avoid having to
continually update tests as we iteratively update the template
- I had to update `karma.conf.js` to get TanStack to work in the unit
tests

## ✅ Checklist

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

Co-authored-by: Jesse Attas <jattasNI@users.noreply.github.com>
Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
This was referenced Dec 12, 2022
Merged
Merged
mollykreis added a commit that referenced this issue Dec 14, 2022
@mollykreis
Create Angular directive for nimble-table (#914)
c1b0926 
# Pull Request

## 🤨 Rationale

Create an Angular directive for the table, so that it can be used within
Angular.

This is part of #840.

## 👩‍💻 Implementation

- Create an Angular directive for the nimble-table that allows binding
data to the `data` property on the table.
- Add a table to the example Angular app.
- Updated component status table to reflect that experimental Angular
support now exists for the nimble-table

## 🧪 Testing

- Create unit tests for the Angular directive following the same pattern
as other unit tests in Angular
- I did not write a test for "with template string values" since I
didn't think it made sense to write a test for hard-coding an array in
the template
- I did not write a test for "with attribute bound values" since `data`
is not an attribute on the `nimble-table` web component

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@nate-ni nate-ni mentioned this issue Dec 15, 2022
Merged
atmgrifter00 pushed a commit that referenced this issue Dec 15, 2022
Table Columns API spec (#900)
8696c6a 
 - #840 

We need a spec for how users will be able to define column views for their data.
mollykreis added a commit that referenced this issue Dec 16, 2022
@mollykreis
Create nimble-table-row and nimble-table-cell and use them in the tab…
1700ba3 
…le (#921)

# Pull Request

## 🤨 Rationale

This PR creates the `nimble-table-row` and `nimble-table-cell` elements,
which will only be used within the `nimble-table`, not as stand-alone
elements exported from `nimble-components`. The property types on these
elements will likely change moving forward, but creating the elements is
a step closer to the correct structure of the DOM and separation of
logic between the table, rows, and cells.

This PR also adds ARIA roles [for a
table](https://w3c.github.io/aria-practices/#table).

This is part of #840.

## 👩‍💻 Implementation

- Create `nimble-table-row`
    - Has ARIA role of `row`
- Responsible for taking in the set of columns and row's record to
create the cells. Currently, the set of columns is specified as an array
of strings, but eventually this will be an array of column definitions
- Create `nimble-table-cell`
    - Has ARIA role of `cell`
- Responsible for rendering one cell of data. Currently this is based on
a single value given to the cell, but eventually this will be based on a
view template and cell data.
- Add ARIA roles to the table & column headers

I think we will also create an element to represent a column header, but
that was not in the scope of this PR.

## 🧪 Testing

- Updated the page object for the table & verified unit tests continue
to pass
- Ensured there are no accessibility warnings/errors in storybook with
the table
- Ensured there are no visual changes introduced with this PR (styling
will come as a separate pass)
- Did not create specific unit tests for the `nimble-table-row` and the
`nimble-table-cell` because we determined in an offline conversation
that they did not seem necessary at this point in time. Until the need
arises, we will plan to test table functionality with unit tests on the
table itself, not its sub-elements.

## ✅ Checklist

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

Co-authored-by: Jonathan Meyer <jonathan.meyer@ni.com>
@mollykreis mollykreis mentioned this issue Dec 16, 2022
Merged
1 task
mollykreis added a commit that referenced this issue Dec 16, 2022
@mollykreis
Table column header component (#928)
1328e1e 
# Pull Request

## 🤨 Rationale

Create `nimble-table-header` component to represent a column header in
the `nimble-table`. Currently it just renders the slotted content, but
it will eventually be extended to include other functionality such as
the drop-down menu for performing actions on the column.

This is part of #840.

## 👩‍💻 Implementation

- Create `nimble-table-header` internal component
- Update table page object

## 🧪 Testing

- Ran unit tests
- Verified that there were no visual changes introduced with this change
- Verified that this change did not introduce accessibility errors

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@mollykreis mollykreis mentioned this issue Dec 16, 2022
Merged
1 task
mollykreis added a commit that referenced this issue Dec 16, 2022
@mollykreis
Basic styling for the nimble-table (#931)
48a9d51 
# Pull Request

## 🤨 Rationale

Basic styling of nimble-table, including:
- Column header fonts
- Padding of cells
- Row height
- Row background color
- Row borders
- Row hover color

This is part of #840 

## 👩‍💻 Implementation

- Used existing tokens where possible but added new tokens for the
column header font and row border color
- We may need to iterate with designers on the row border color on the
'Color' theme. Right now it is the same color as the background of the
row.

## 🧪 Testing

- Verified the styling looked correct in storybook

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@atmgrifter00 atmgrifter00 mentioned this issue Dec 19, 2022
Merged
@nate-ni nate-ni mentioned this issue Jan 3, 2023
Closed
@nate-ni
Copy link
Contributor

nate-ni commented Jan 4, 2023

@RickA-NI, can you verify that the table implementation matches the visual design spec?
You can see an example of the table at: https://nimble.ni.dev/storybook/?path=/story/table--table

@mollykreis, do we need to validate the Angular and Blazor (when ready) implementations as well or is the code shared enough that they will be identical?

@mollykreis
Copy link
Contributor

do we need to validate the Angular and Blazor (when ready) implementations as well or is the code shared enough that they will be identical?

@nate-ni, there is no need to validate the styling in Angular and Blazor. All the styling is shared.

atmgrifter00 pushed a commit that referenced this issue Jan 4, 2023
Blazor nimble table (#932)
5a509a2 
- #840
@nate-ni nate-ni moved this from In progress to Pending in Nimble Design System Priorities Jan 5, 2023
@RickA-NI
Copy link

RickA-NI commented Jan 6, 2023 •
edited
Loading

The Light skin and Dark skin look correct, but something is off on the color skin. The lighter background color for the rows and headers is not showing up all I'm seeing is text on the dark green.

@mollykreis
Copy link
Contributor

@RickA-NI, we were unsure the correct way to style the Color theme. We were confused because the dividers between the rows was speced to be ForestGreen@75% while the rows themselves are ForestGreen@100%. This means that the dividers are only visible on some backgrounds. Could you clarify what color the dividers should be on the Color theme? ForestGreen@75% layered on top of some other color (that at least I couldn't find in the XD document)?

If the answer isn't straight forward, we can follow up in person.

@RickA-NI
Copy link

@mollykreis looks like that was due to poor design on my part. I've updated the color page of the spec to match how the other color controls are styled. It should work properly now. If there's questions let me know.

@nate-ni nate-ni moved this from Pending to Done in Nimble Design System Priorities Jan 18, 2023
@nate-ni nate-ni moved this from Done to Pending in Nimble Design System Priorities Jan 18, 2023
@nate-ni
Copy link
Contributor

nate-ni commented Jan 18, 2023

@nate-ni nate-ni closed this as completed Jan 18, 2023
@nate-ni nate-ni moved this from Pending to Done in Nimble Design System Priorities Jan 18, 2023
@nate-ni nate-ni linked a pull request Jan 27, 2023 that will close this issue
Basic TanStack integration with nimble-table #902
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mollykreis mollykreis

@atmgrifter00 atmgrifter00

Labels
new component Request for a new component
Projects
Nimble Design System Priorities
Status: Done
Milestone
Table Milestone 1
Development

Successfully merging a pull request may close this issue.

Basic TanStack integration with nimble-table ni/nimble
4 participants
@mollykreis @atmgrifter00 @RickA-NI @nate-ni