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

Datagrid: Default column width renders too wide #4016

Closed
pwpatton opened this issue Jun 5, 2020 · 22 comments · Fixed by #4185
Closed

Datagrid: Default column width renders too wide #4016

pwpatton opened this issue Jun 5, 2020 · 22 comments · Fixed by #4185
Assignees
Labels
focus: datagrid Main focus is Datagrid team: landmark For Landmark issues type: bug 🐛 [5] Velocity rating (Fibonacci)

Comments

@pwpatton
Copy link
Contributor

pwpatton commented Jun 5, 2020

Describe the bug
One pushback we are getting in landmark for 4.x is that there is too much horizontal scrolling for datagrids compared to the 3.5.

To Reproduce
Steps to reproduce the behavior:

  • See me for how URL/login and credentials.

Expected behavior
Want to columns to be greedy about reclaiming white space.

Version
7.1.1 of ng/ 2.28 of ep
Screenshots
In this case many columns seem to take more space than is needed. Some have their filter fields taking up more width than their cell.
image
In 3.5 this was much less of an issue because in landmark we set the size to fit on the columns and had a much smaller font.
image
A more aggressive reclaim of white space could be of at least marginal improvement.
image

Platform

  • PC
  • Window10
  • Latest Chrome

Additional context

Add any other context about the problem here.

@tmcconechy tmcconechy changed the title Default column widths are still to wide. Datagrid: Default column widths are still to wide. Jun 5, 2020
@tmcconechy tmcconechy self-assigned this Jun 5, 2020
@tmcconechy tmcconechy added [3] Velocity rating (Fibonacci) focus: datagrid Main focus is Datagrid type: bug 🐛 labels Jun 5, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.30.x (June 2020) Sprint via automation Jun 5, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.31.x (July 2020) Sprint via automation Jun 5, 2020
@tmcconechy tmcconechy moved this from To do to In progress in Enterprise 4.30.x (June 2020) Sprint Jun 10, 2020
@tmcconechy
Copy link
Member

@pwpatton can you maybe give me the JSON data, grid settings and column settings for this grid example? I think i could make an example out of that.

@tmcconechy
Copy link
Member

tmcconechy commented Jun 17, 2020

So i studied 3.5 the difference is that it doesn't fill the grid area. It just sizes the columns to the data and stops. Do we want that? Leaving a space on the left?

This could be an option, we just add an empty column at the end. But with less columns it wont evenly spread.

Screen Shot 2020-06-17 at 2 50 08 PM

Do we want an option for spaceColumn? Or just a shorter defaulting of column sizes and still stretching it if need be?

Screen Shot 2020-06-17 at 2 53 13 PM

@tmcconechy
Copy link
Member

Also #3755 will record quite a bit of space by just changing the font size and margin

@pwpatton
Copy link
Contributor Author

Can we fit as best as possible with little white space when the the container is narrow but spread them out if the container is large enough?

@tmcconechy
Copy link
Member

The problem is really only when there is less columns and space than the datagrid width. At that point it goes and stretches the columns out across overriding whatever you set for widths so at that point the columns get big enough to stretch, stretching each one a bit beyond what it would be at the narrow size.

So i can just set the initial "narrow" size but once the grid is much wider than all the defaults the begin to stretch across. Then the only way is to make an extra blank stretching column

I.E. We are partially stuck by the table / browser it has a specific algorithm we cant override. https://www.w3.org/TR/CSS21/visudet.html#propdef-max-width things like maxwidth wont work, it just gets stretched.

Ok, what i will do is look for some tweaks on the size. Making it the size of number of characters in the filter container. Should reclaim some. Then can also add the spacer column and you can try it out

@tmcconechy tmcconechy added [5] Velocity rating (Fibonacci) and removed [3] Velocity rating (Fibonacci) labels Jun 17, 2020
@pwpatton
Copy link
Contributor Author

I think we could deal with a spacer column if it looked more like the other cells. And by spacer I mean one at the end that isn't one of the data columns. When it cuts off like before customer really didn't like it.

@tmcconechy
Copy link
Member

yeah would just be black space on the header and then the row would continue . i'll work up some options on this.

@tmcconechy tmcconechy moved this from In progress to To do in Enterprise 4.30.x (June 2020) Sprint Jun 23, 2020
@tmcconechy tmcconechy added the team: landmark For Landmark issues label Jun 23, 2020
@tmcconechy tmcconechy moved this from To do to In progress in Enterprise 4.30.x (June 2020) Sprint Jun 26, 2020
@tmcconechy tmcconechy moved this from In progress to Pending Review in Enterprise 4.30.x (June 2020) Sprint Jun 26, 2020
@tmcconechy tmcconechy moved this from Pending Review to In progress in Enterprise 4.30.x (June 2020) Sprint Jun 26, 2020
@tmcconechy
Copy link
Member

@pwpatton just so im comparing apples to apples. Can you send me the JSON data for that "books" page. Would need both the columns and dataset data.

Thanks!

@lipetzan
Copy link
Collaborator

lipetzan commented Jul 9, 2020

Woah, this was pushed out to 31? Can we talk?

@tmcconechy
Copy link
Member

tmcconechy commented Jul 9, 2020

Yes, we can talk. It was too much work/plus other things added. Had to push. But we did do #3995 which does help a lot i think.

@tmcconechy
Copy link
Member

@pwpatton
Also Still looking to get this data (columns and data for this example) ...to accurately test and start this. Or at least either the columns or dataset?

@tmcconechy tmcconechy moved this from To do to In progress in Enterprise 4.31.x (July 2020) Sprint Jul 9, 2020
@tmcconechy tmcconechy changed the title Datagrid: Default column widths are still to wide. Datagrid: Default column width renders too wide Jul 10, 2020
@tmcconechy
Copy link
Member

tmcconechy commented Jul 10, 2020

Found two problems so will adjust this.

  1. if on short row height its sizing as if the padding was "normal" - extra 16px per cell. this is maybe a helpful fix
  2. its giving too much space for the filter row

@tmcconechy
Copy link
Member

Screen Shot 2020-07-10 at 4 49 41 PM

BTW dont need the data per se anymore i made an example

@tmcconechy tmcconechy moved this from In progress to Pending Review in Enterprise 4.31.x (July 2020) Sprint Jul 23, 2020
@tmcconechy tmcconechy moved this from Pending Review to Ready for QA (beta) in Enterprise 4.31.x (July 2020) Sprint Jul 28, 2020
@janahintal
Copy link
Contributor

The default width still renders wide for some columns that do not have long data although there has been a size reduction.

image
image

@tmcconechy
Copy link
Member

This is correct @janahintal (or at least what i was trying to do).

https://4310-beta1-enterprise.demo.design.infor.com/components/datagrid/example-books.html -> this page has a small amount of reduction and sizes including the header

https://4310-beta1-enterprise.demo.design.infor.com/components/datagrid/example-columnsizing.html -> is the same xample with data size only truncating the header

@pwpatton
Copy link
Contributor Author

pwpatton commented Aug 3, 2020

@tmcconechy is't the idea that we still need a min size for the filter bar so that filter fields don't get truncated out of view. I think one thing that could improve that as well is to only show the down arrow and not the icon in the data case.

@tmcconechy
Copy link
Member

I tried to just keep it consistent to one character for the filter. After all it was emphasizing the data. So that should be close. Also its pretty hard to makeout the header text unless its 5-6 characters

@claudenbach
Copy link
Contributor

Let me know when a package is ready with this change.

@tmcconechy tmcconechy moved this from Ready for QA (beta) to Ready For QA (RC) in Enterprise 4.31.x (July 2020) Sprint Aug 3, 2020
@pwpatton
Copy link
Contributor Author

pwpatton commented Aug 3, 2020

@tmcconechy @claudenbach isn't this already in 7.5.0-beta.2?

@tmcconechy
Copy link
Member

tmcconechy commented Aug 3, 2020

No i didnt make a beta 2 yet as i was waiting for our testing to proceed further. (Cant make a build for every issue).
But its ready now so there will be a 7.5.0-rc shortly today

@tmcconechy
Copy link
Member

Ok made a mistake but its ready. There is a 7.5.0-rc1 which has 4.31.0-rc.0

@janahintal
Copy link
Contributor

Thank you guys for the confirmation. Working fine on my end & will now move this to done.
Retested it in https://4310-beta1-enterprise.demo.design.infor.com/components/datagrid/example-columnsizing.html & https://4310-rc0-enterprise.demo.design.infor.com/components/datagrid/example-books.html

image
image

@janahintal janahintal moved this from Ready For QA (RC) to Done in Enterprise 4.31.x (July 2020) Sprint Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: datagrid Main focus is Datagrid team: landmark For Landmark issues type: bug 🐛 [5] Velocity rating (Fibonacci)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants