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

Fix the ngReactGridCheckbox column default width #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pmq
Copy link
Contributor

@pmq pmq commented Nov 17, 2014

Like #33, not sure if it is only on my browser, but the default checkbox column width is too small. Feel free to close abruptly if I'm the only one :)

Tested using a grid setup like this :

      $scope.grid = {
          data: [],
          columnDefs:[
              new ngReactGridCheckbox($scope.redactedSelected, { batchToggle: true }),
              { field: 'Time', displayName: 'Date',
...

Before :
checkbox-column-width

After :
checkbox-column-width-corrected

@vladud
Copy link

vladud commented Nov 17, 2014

It's not the best solution, if there are a small number of columns then the select column would be huge for no reason.
Maybe a "1%" or "2%" value would be better, but still not good enough...
It's not a problem when you know the number of columns and you can change the value to what works for you, but when the no of columns change dynamically...

@pmq
Copy link
Contributor Author

pmq commented Nov 17, 2014

Hi @vladud, you're right it's not an exact fix, but I thought it would be a saner default value. I think the percentage one is even more error-prone.

In that case, maybe we could change ngReactGridCheckbox with a syntax like this : new ngReactGridCheckbox($scope.expensesSelected, { batchToggle: true, width: '20' }) to let everyone override the width without having to subclass? Or maybe it's already there and I'm misreading the source; how do you set it?

@vladud
Copy link

vladud commented Nov 17, 2014

No, I don't think you can set it that way, it would be ok to export the setting.. Personally I preferred to implement my own selection without a checkbox column, but it's a quick and dirty implementation .. I've only used the library for a few tests so far

@josebalius
Copy link
Owner

@pmq @vladud so what do you guys think we should do to deal with this?

@pmq
Copy link
Contributor Author

pmq commented Nov 17, 2014

I'm mentioning @ewu02 also, who can probably provide more informed feedback. From my point of view, we should :

  • provide a sane default (be it 20, a percentage, something else)
  • provide a way to override easily; I think the syntax I described using options would feel natural

I can provide a patch if we agree on that, and probably also provide an example for documentation.

@vladud
Copy link

vladud commented Nov 17, 2014

Yeah, that seems ok. Maybe you could also change the default "10%" cell width to 100% / noOfColumns ... I guess that would provide the perfect fit. (no of column except select column)
Actually changing it to "100%" would work the same way :/

@ewu02
Copy link
Contributor

ewu02 commented Dec 1, 2014

Just recently encountered the same issue, which seems to show up when there are many columns. I would favor having an override option with % over table width (or noOfColumns), with the default being 10% of table width as well.

@ewu02
Copy link
Contributor

ewu02 commented Jan 15, 2015

Created pull request #60 that should solve this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants