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

New behaviour in List component #119

Open
5 tasks
Albertlarsen opened this issue Sep 13, 2022 · 9 comments
Open
5 tasks

New behaviour in List component #119

Albertlarsen opened this issue Sep 13, 2022 · 9 comments
Assignees
Labels
team/auth Issue affecting the auth team

Comments

@Albertlarsen
Copy link
Contributor

Albertlarsen commented Sep 13, 2022

We need some new behaviors for List component that is described in https://www.figma.com/file/vpM9dqqQPHqU6ogfKp5tlr/DDS---Core-Components?node-id=6632%3A25291. Implement the relevant behaviours. It should be able to choose if the component is going to have hover effect and not.

This issue is only about making these follwing behaviours:

  • Default
  • Hover
  • Focus
  • Hover warning
  • Strikethrough
@Albertlarsen Albertlarsen self-assigned this Sep 13, 2022
@Albertlarsen Albertlarsen added the team/auth Issue affecting the auth team label Sep 13, 2022
@Albertlarsen Albertlarsen changed the title List component | New behaviour New behaviour in List component Sep 13, 2022
@olemartinorg
Copy link
Contributor

From the looks of it, these 'lists' in figma optionally has multiple columns, and such they would map to what we colloquially call a 'table'. Which is already implemented, albeit with a slightly different design. See here for an example.

Please expand on what exactly is needed from the figma sketches and is not implemented already. Should we perhaps support multiple visual variants? If so, we should probably describe when to use which.

Also relevant:

@Albertlarsen
Copy link
Contributor Author

@olemartinorg I don't understand what you mean. What does Table have to do with the List component?

@olemartinorg
Copy link
Contributor

The list component you linked in figma looks suspiciously like a table, so I believe there to be overlapping functionality between them. I would suggest looking into that overlap, and consider if a new list component really should be implemented, or if the table component could be extended to more easily allow for the features required in this list component.

FYI, the table component here is an external contribution made to support the so-called List component in app-frontend-react. It is overlapping, not only in functionality, but also in naming.

From what I can tell by glancing at the figma sketches, it looks like the list component we're talking about here is a HTML table, which allows for some icons and right-aligned text inside some cells, and has some functionality where if you click on a row, some content appears below that row in what looks like a <tr><td colspan="X">content</td></tr> (where X is the number of columns in the table above). That could easily be added as a feature to the existing table component, IMO.

@Albertlarsen
Copy link
Contributor Author

Albertlarsen commented Jan 12, 2023

@olemartinorg Yeah those things you meantion you have to talk to @hannekristin about, as she has made the design in figma. To make sure we're on the same page I just want to mention that this issue is really just about creating an extended behaviour to the current List component in the Design System.

Regarding if this should really be the same component or 2 components. I at least see one problem with merging the 2 components. F.ex that html distinguishes between tables(< table >, < tr > and < th >) and lists(< ul > and < li >). I guess you can debate that a list is just an table without structure(but in authorization we need a component like that) but at least html has distinguished this to be 2 completely different html elements.

I'm not negative to create a component a bit like _InputWrapper that can be used by both Table and List to gain the behavior described in this issue though.

List component was made before Table component and I don't really have to do anything with the architecture of Table and the thought process behind ignoring List component. List component was merged into the Design System 4th August and it seems like the work on Table component was started Sept 23.

@Albertlarsen
Copy link
Contributor Author

@olemartinorg Also I'm wondering why they are calling it a List component in List component in app-frontend-react when it really is a Table?

@olemartinorg
Copy link
Contributor

Thank you, this explains a lot! IMO, I still think the more advanced use-cases i see in the List component in Figma looks more like a table. I didn't see any specifics here, only a link to figma and a one-liner about implementing some behaviour I interpreted as "the rest of it", so forgive me if this issue only meant to describe hover- and selection-effects.

As for the naming of Table -> List, the naming was chosen because the use-case is "choose this item from a list", and it more closely resembles a dropdown or radio group in functionality. Also, historically, Table is a tainted term in our heads because of #107 which describes functionality that spans both this aforementioned List component, and repeating groups (which now also uses the Table component from the design system under the hood). The dreamed-up Table component which was wanted in #107 has thus been split into 3 parts (the List component, a Grid component and further improvements to the existing repeating groups). Read more about the history here.

@Albertlarsen
Copy link
Contributor Author

@olemartinorg Why does it look more like a table to you? 😊

@olemartinorg
Copy link
Contributor

Screenshot from figma of a list example:

20230112_16h52m20s_grim

The Table component:

20221013_21h57m08s_grim

So, because there are:

  • Rows
  • Columns
  • Cells

In both of these, I am of the impression they can (and should) be represented using the table abstraction.

@Albertlarsen
Copy link
Contributor Author

Albertlarsen commented Jan 12, 2023

Yes for that use case we could maybe just use a table without a header. You have to discuss those UX cases further with @hannekristin 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team/auth Issue affecting the auth team
Projects
None yet
Development

No branches or pull requests

2 participants