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

Define column widths on both Col and TableList component #129

Closed
happsie opened this issue Jan 5, 2020 · 4 comments · Fixed by #139
Closed

Define column widths on both Col and TableList component #129

happsie opened this issue Jan 5, 2020 · 4 comments · Fixed by #139
Labels
help wanted Extra attention is needed new feature New feature or request

Comments

@happsie
Copy link
Contributor

happsie commented Jan 5, 2020

Is your feature request related to a problem? Please describe.
I think you should have the possibility to change the width of the columns, especially in the TableList component. When you many column in the list, some may just contain one character and some might have a lot of information. It makes no sense that the column with one character should take as big of a space like the one with a lot of information and get cropped to a new line. Instead we should be able to manage this a bit to free up more space for the ones needed.

Describe the solution you'd like
Maroto should be able to specify not only height, but also width like so:
m.Col(20,19, func() {})

The TableList should maybe have a slice of float64 in the TableProps defining each columns width.

Describe alternatives you've considered
Add additional but optional extra parameter / rewrite to a struct for the Col component to take in.
Add extra struct field called ColSizes that is of type []float64 that holds the size of the columns.

@happsie happsie changed the title Define column widths on both Col component and TableList Define column widths on both Col and TableList component Jan 5, 2020
@johnfercher johnfercher added help wanted Extra attention is needed new feature New feature or request labels Jan 5, 2020
@happsie
Copy link
Contributor Author

happsie commented Jan 9, 2020

@johnfercher do you think this one is hard to solve?

@johnferchermeli
Copy link
Contributor

johnferchermeli commented Jan 9, 2020

I think this is a very good feature, but we have to analyze the possibilities first. These are my thoughts about it.

  1. I don't give the possibility to define the width of each col. If the user set the width with the pixel value, what happens if the sum of the cols pass the useful area of the PDF?
  2. Given that maroto ins inspired in Bootstrap, I think we should give the possibility to set a value between 0 or 12 to each col (as Bootstrap does), and if sum of cols pass 12 in one Row I think we should apply the same logic that Bootstrap apply.
  3. I don't think this is too difficulty.

This is the first thing you should see, this code is used in many places to calc the width of the cells. Maybe this will have to consider the actual width value of each col.

maroto/internal/math.go

Lines 29 to 33 in 09beb58

func (s *math) GetWidthPerCol(qtdCols float64) float64 {
width, _ := s.pdf.GetPageSize()
left, _, right, _ := s.pdf.GetMargins()
return (width - right - left) / qtdCols
}

@happsie
Copy link
Contributor Author

happsie commented Jan 9, 2020

My first thought was to work with percentage to handle the amount of columns not overriding page width. But I think the idea of working with a 12-col grid is better, as it also is in line with bootstrap. I think this should be a new Col component because I think the value of the current is still very high and good for most cases.

I will try implement this and ask you for help if I get stuck on something. 😄

@johnferchermeli
Copy link
Contributor

Nice, it must be a Col feature. But you will need to change the code above (GetWidthPerCol).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants