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

Migrate DoubleRow and TableMenu components from maas-ui #380

Open
huwshimi opened this issue Jan 26, 2021 · 8 comments
Open

Migrate DoubleRow and TableMenu components from maas-ui #380

huwshimi opened this issue Jan 26, 2021 · 8 comments
Labels
P3 Needs decision Needs team agreement in terms of triaging. Priority: Medium

Comments

@huwshimi
Copy link
Collaborator

This should be made to work with ModularTable.

@anthonydillon
Copy link
Contributor

@huwshimi you mentioned there needs to be a discussion on cell generation before this can happen. Would it be possible for you to put forward a proposal of how you think it would work?

@huwshimi
Copy link
Collaborator Author

@huwshimi you mentioned there needs to be a discussion on cell generation before this can happen. Would it be possible for you to put forward a proposal of how you think it would work?

Sure, I'll take a look and reply here with the details.

@huwshimi
Copy link
Collaborator Author

Initially I thought we'd have to generate the row objects, but I think instead we should:

  • Remove icon support from the existing (maas-ui) double row.
  • Update ModularTable with a getHeaderIcon prop to support showing icons in the header rows.
  • Update ModularTable to support showing spinners in the icon spot (current it can only show an <Icon />).
  • Create a modular table class to support showing an icon and block content on the same line.

I have branch that implements about half of this which I can keep tinkering with when I have some time.

@bartaz
Copy link
Member

bartaz commented Feb 12, 2021

@huwshimi Thanks for the summary. I don't know the exact context of the double row in maas-ui, but from the things that you mention it seems like there is a lot about icons, and not much about double rows. :)

Could you throw some screenshots into this issue to show what kind of content/icons need to be implemented in modular table?
You seem to mention icons in header, spinner, etc. Would be nice to have images of those to better understand what we want to achieve.

This will also let me better understand the needs so I can try to help suggest how to implement in in ReactTable (at least as much as I remember their API).

@huwshimi
Copy link
Collaborator Author

@bartaz here's how they look in MAAS. We also sometimes have checkboxes in the icon slot (and then sometimes have nested checkboxes like in this screenshot).
Screen Shot 2021-02-15 at 10 45 52 am

@bartaz
Copy link
Member

bartaz commented Feb 15, 2021

@huwshimi

Thanks. It seems to me that the checkbox is not an icon, it's a selectable row which is a separate and different feature that would need to be ported to modular table: https://discourse.ubuntu.com/t/row-selection/19847

From implementation point of view ReactTable has a hook for row selection, hopefully we should be able to use it as is: https://react-table.tanstack.com/docs/examples/row-selection

As for the 'nested' rows, it seems that this is MAAS way of doing grouping of the table rows. And as far as I understand the modular table is not going to do the grouping by nesting, it's supposed to be more like RBAC style: https://discourse.ubuntu.com/t/grouping/19857

ReactTable has a support for grouping (https://react-table.tanstack.com/docs/examples/grouping) by the value of a column (which seems to be what we need), so maybe we could use that. Although their example looks a bit different from our spec (they have a 'grouping' row that can be expanded/collapsed), but hopefully we could use their grouping hook implementation for data manipulation, but still render it as we need.

@bartaz
Copy link
Member

bartaz commented Feb 15, 2021

As for the double row implementation, ReactTable allows a lot of customisation about how cells are rendered. By default the column options (https://react-table.tanstack.com/docs/api/useTable#column-options) have accessor field which tells which field from the data object should be rendered by Cell component. Both can be overridden (although accessor is also used for sorting and other things, and serves a bit as and identifier of the row, so hacking it needs to be done with care).

So I think the easiest way to implement double rows would be to implement custom DoubleCell component that would render two values instead of one.
This seems to be giving a trivial example of just concatenating two values:

Cell: row => ( return ${row.original.buyer} ${row.original.buyerPurhcaseId} )

But also maybe we can be more smart and not hardcode the data fields, but pass some configuration via custom column props or something...

@huwshimi
Copy link
Collaborator Author

I just pushed up my branch from a while ago where I started this work:

https://github.com/canonical-web-and-design/react-components/compare/master...huwshimi:add-double-row?expand=1

It looks like it contains some custom CSS, but it seems we no longer include CSS in react-components so I guess we need to upstream the double row and table menu styles first:

https://github.com/huwshimi/vanilla-react-components/blob/0adba4cf416b9d733bfe7908e66e129e33a0adc4/src/components/TableMenu/_index.scss

@bartaz bartaz added the P3 Needs decision Needs team agreement in terms of triaging. label Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Needs decision Needs team agreement in terms of triaging. Priority: Medium
Projects
None yet
Development

No branches or pull requests

3 participants