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

ui.table enhancements + redesign #974

Closed
iamabhishekmathur opened this issue Aug 19, 2021 · 29 comments · Fixed by #989
Closed

ui.table enhancements + redesign #974

iamabhishekmathur opened this issue Aug 19, 2021 · 29 comments · Fixed by #989
Assignees
Labels
feature Feature request ui Related to UI

Comments

@iamabhishekmathur
Copy link

Table View

image

Features

  • Columns should be sortable
  • Should allow for 'badges' as part of field values
  • Should have hover over effect
  • Clicking on a row should be evented

Note: don't need the "Action" column equivalent

@iamabhishekmathur
Copy link
Author

cc: @sandruparo @dulajra @mwysokin

@mturoci mturoci added the ui Related to UI label Aug 20, 2021
@mturoci mturoci changed the title New Component: Table ui.table enhancements + redesign Aug 20, 2021
@mtanco
Copy link
Contributor

mtanco commented Sep 7, 2021

@mturoci here is what I see as the major changes from current ui.table object:

  • Overall style / format
  • Style on hover
  • "badges" inside of table cells - @mturoci I don't believe there is currently a badges component, should this be a separate request?
  • Hover text per row

Related UI Table Requests to consider implementing during this component redesign :

@mturoci
Copy link
Collaborator

mturoci commented Sep 7, 2021

I don't believe there is currently a badges component, should this be a separate request

I can do this as part of this issue by introducing a new table_cell_type (like we have icon, progress).

Hover text per row

Not sure about this one, do you want to have a tooltip displayed when hovered over row? Can you show an example usecase maybe?

The rest of requests seems reasonable for me, thanks @mtanco!

@mtanco
Copy link
Contributor

mtanco commented Sep 7, 2021

Oops, I misunderstood. Just a hover effect rather than hover text. The Table component already does this so it is just a design update.

@mturoci mturoci self-assigned this Sep 8, 2021
@mturoci
Copy link
Collaborator

mturoci commented Sep 8, 2021

Redesign so far:
image
image

A few design questions - cc @sandruparo @shanaka-rajitha @shihan007 :

  • row border color is probably too low contrast in the dark theme - needs work ^^
  • image Remember that not all columns always have an icon. This inconsistency can cause the col alignment to data look funky - data is sometimes aligned to text, sometimes to icon. I suggest sticking with a current right alignment which makes sure the data is always aligned to column text.
  • Also it would be good if the designs could use Fluent icons. For example Fluent's sorting icon might be even better than the proposed one UXwise as it indicates sort direction as well.
  • image - What is the purpose of this? We currently support either normal data display (see pics above) or grouped mode (see below). Not sure where this belongs or what the usecase is.

Currently, the grouped mode looks like this:
image
image
The design however proposes header duplication which I think is unnecessary as the header is always the same for all the data.
image

Design team, please let me know what you think.

@lo5 here is a proposal for badges table col cell API:

/** Defines cell content to be rendered instead of a simple text. */
interface TableCellType {
  /** Renders a progress arc with a percentage value in the middle. */
  progress?: ProgressTableCellType
  /** Renders an icon. */
  icon?: IconTableCellType
  /** Renders a one or more chips (badges). */
  badge?: BadgeTableCellType
}

/** 
 * Creates a collection of chips, usually used for rendering state values.
 * Note: In case of multiple tags per row, make sure the row values are
 * separated by "," within a single cell string.
 * E.g. ui.table_row(name='...', cells=['cell1', 'BADGE1,BADGE2']).
 */
export interface BadgeTableCellType {
  /** An identifying name for this component. */
  name: S
  /** Badges to be rendered. */
  badges?: Badge[]
}

/**
 * Create a badge.
 */
export interface Badge {
  /** Text specified within the badge. */
  name: S
  /** Badge's background color. */
  background_color: S
  /** Badge's text color. If not specified, black or white will be picked based on correct contrast with background. */
  color?: S
}

The problem is that we want to support multiple badges per col. The only way to achieve this I could come up with was requiring people to serialize multiple badges within a single string. , is currently used as delimiter, but this can be configurable if needed ofc. Second problem is in determining which color belongs to which badge. For this I proposed specifying a list of badges that serves as a Map to get the correct badge config. Wdyt?

@shihan007
Copy link

shihan007 commented Sep 8, 2021

@mturoci for table I have used neutralLight color for header and border did you use the same?
https://xd.adobe.com/view/0790f950-abbd-41fa-b372-332295fd876f-52c3/screen/4ef1b0c3-50cc-4782-9400-193c438cdc41/

@mturoci
Copy link
Collaborator

mturoci commented Sep 8, 2021

Yes, that's the one I used. Same as header. The contrast is too low also in the design link posted.

@shihan007
Copy link

@mturoci okay then for borders lets go with neutralTeritaryAlt. I will check on icons

@shihan007
Copy link

@mturoci okay then for borders lets go with neutralTertiaryAlt. I will check on icons

@shihan007
Copy link

@mturoci can we change the placement of sort icon to left if we using the fluent icon? because some column header may have sort and filter dropdown too..

@mturoci
Copy link
Collaborator

mturoci commented Sep 9, 2021

okay then for borders lets go with neutralTeritaryAlt.

Not much of a difference.
image
I think we should probably increase the border width. I noticed that XD is somewhat not so exact on rendering the correct px sizes. They appear fine in XD, but in reality, they are much smaller.

Here is a 2px border width with original neutralLight color:

image

wdyt @shihan007 ?

can we change the placement of sort icon to left if we using the fluent icon? because some column header may have sort and filter dropdown too.

What's the reasoning? As I described in a comment, that would break alignment with data.

@shihan007
Copy link

@mturoci For some column headers sort and filter both needed in that case better to place sort icon to left. And each and every column does come with sort feature right?

@mturoci
Copy link
Collaborator

mturoci commented Sep 9, 2021

For some column headers sort and filter both needed in that case better to place sort icon to left.

But why placing it to the left? What would be the benefit?

And each and every column does come with sort feature right?

Every column can be specified as sortable / filterable, that's right.

@sandruparo
Copy link

@mturoci I agree with @shihan007 Shihan. I think it's good to place the sorting option left align. anyways most of all the tables in the wave app store needs tables with sorting options. So i dont think it's funky if we can align it nicely to the icon.cause icon should always be there.

and telling about the rest of the implementation you did, - it's great Martin. great improvement. But i still see many color difference and spacing issues compared to the design.

image
image

  1. Scroller
  2. Row height
  3. Row highlight colors ( wondering whether you apply the theme Shihan has given you )
  4. Font Colors
  5. Overall i feel we need to match everything with the colors Shihan gave you.

Functionality wise i feel you have achieved almost it. But i see look and feel is different still. I just want to know is it possible to update it's theme ?

@mturoci @mtanco

@mturoci
Copy link
Collaborator

mturoci commented Sep 9, 2021

So i dont think it's funky if we can align it nicely to the icon.cause icon should always be there.

This is not true. See the example above for example. Making Done column sortable would make no sense. Also the column sortability is determined based on whether it makes sense for the concrete column or not (the app dev decides).

Besides that, you still didn't provide a reason why left is better than right.

Scroller

This is a default browser scrollbar for overflowing content. Some browsers support styling it if you mean this, but the browser support is not great at all thus going for the default seems the most reasonable in order to deliver the same experience across multiple browsers.

Row height

This one is dynamic based on data and colum types (e.g. in this case progress table cell type makes the whole row bigger). I can set min-height to the one used in designs (48px) though, good point.

Row highlight colors ( wondering whether you apply the theme Shihan has given you )

Yes, these are exactly the colors from the theme. Note that the theme in the screenshot is our current neon theme if you are wondering why the there is green instead of yellow. As we have discussed countless times, we need the designs to be working with any theme provided so concrete colors don't matter (e.g. whether the color is green or yellow in this case). The important thing is that primary color is used and the contrast is good enough.

Also note about the first column. This serves as clickable link in our table so that's why the color is different from the other cols.

@sandruparo
Copy link

sandruparo commented Sep 9, 2021

  1. This is not true. See the example above for example. Making Done column sortable would make no sense. Also the column sortability is determined based on whether it makes sense for the concrete column or not (the app dev decides). -

---- Why do we need a " Done " column ? if we need to show the user an indication the particular work in the row is done or some states , in UX perspective we have given more suggestions for that. I dont think we need a column like that ?

  1. Also the column sortability is determined based on whether it makes sense for the concrete column or not (the app dev decides)

----- Agree. Sure we can give a personalised configuration to the user to define those , t's setting in the fronted or backend ?

  1. This is a default browser scrollbar for overflowing content. Some browsers support styling it if you mean this, but the browser support is not great at all thus going for the default seems the most reasonable in order to deliver the same experience across multiple browsers.

----- i would like to have a scroll defined to us, an unique one based on the design ( those requirements are not major ones, maybe in the future )

  1. This one is dynamic based on data and colum types (e.g. in this case progress table cell type makes the whole row bigger). I can set min-height to the one used in designs (48px) though, good point.

---- Please add a minimum height. yes.

  1. Yes, these are exactly the colors from the theme. Note that the theme in the screenshot is our current neon theme if you are wondering why the there is green instead of yellow. As we have discussed countless times, we need the designs to be working with any theme provided so concrete colors don't matter (e.g. whether the color is green or yellow in this case). The important thing is that primary color is used and the contrast is good enough.

-- GREAT ! But i would like to see how this will look when we add our YELLOW theme. Cause Right now it matters to me a lot, we as the design team want to see it's look and feel. for the countless time im requesting on that too. 🤓 🤷‍♀️🙏

@mturoci
Copy link
Collaborator

mturoci commented Sep 9, 2021

---- Why do we need a " Done " column ? if we need to show the user an indication the particular work in the row is done or some states , in UX perspective we have given more suggestions for that. I dont think we need a column like that ?

I am talking about icons columns in general. The point stands, it does not make sense to make each column sortable.

----- Agree. Sure we can give a personalised configuration to the user to define those , t's setting in the fronted or backend ?

Wave is backend only that allows you to draw a UI. The config is already in place. The comment was about the sorting icon placement.

----- i would like to have a scroll defined to us, an unique one based on the design ( those requirements are not major ones, maybe in the future )

Not sure I understood, what definitions do you expect?

See my comment about browser support (this means the scrollbar will not be the same across multiple browsers / browser versions so 2 different users might end up with differently looking UI). IMO consistency across browsers (for every user) > multiple variations for scrollbar styles or do you have something else in mind?

-- GREAT ! But i would like to see how this will look when we add our YELLOW theme. Cause Right now it matters to me a lot, we as the design team want to see it's look and feel. for the countless time im requesting on that too. 🤓 🤷‍♀️🙏

I can send you a screenshot with the theme from designs although this will not eliminate the need for changes requested above so I don't see much value in it.

Here you go:
image

@shihan007
Copy link

@mturoci can we change hover bg to neutralLight and do the adjustment on spacing as @sandruparo mentioned.

@mturoci
Copy link
Collaborator

mturoci commented Sep 9, 2021

Sure, done. By spacing you mean min row height right?

@shihan007
Copy link

@mturoci yeah row height. Can't we have set a row height to a fixed value?

@mturoci
Copy link
Collaborator

mturoci commented Sep 9, 2021

Not really, see my comment above.

If set statically, it would look like this. We also might support longer text with wrapping in future, so static height would not work there.

Setting to 48px looks like this (progress arc overflows)
image

But no worries, without the progress it has 48px height + 2px border:
image

@shihan007
Copy link

@mturoci table looks great now. you working separately on badges? we have some changes in badges.

@sandruparo
Copy link

@mturoci looks good now. Thanks for the efforts, i like how it looks now. Can we play with badges colors ?

@mturoci
Copy link
Collaborator

mturoci commented Sep 9, 2021

Badges colors are configurable - any color can be specified.

Any news about group by table designs I wrote about above?

@sandruparo
Copy link

Yes that also i read @mturoci - grouping is fine but can we have nested groups within another group sets ? Like a tree hierarchy? If this component supports that and if it can has the style we gave we are fine.

Well and how about drawing those lines ? hierarchy lines ?

mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
mturoci added a commit that referenced this issue Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request ui Related to UI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants