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

Old column widths are reused on matching column names #3880

Closed
Jeff-Clark opened this issue Jun 26, 2015 · 4 comments
Closed

Old column widths are reused on matching column names #3880

Jeff-Clark opened this issue Jun 26, 2015 · 4 comments

Comments

@Jeff-Clark
Copy link

A very slight modification to the 112 tutorial (add/delete/swap data) illustrates the problem.
In colDefs1 the 'company' field has width:60 and it works fine. colDefs2 has a column defined with the same name but with width:120 . When you swap the data to use colDefs2 it keeps the width of the company column at 60 rather than using the new value of 120.

It looks like there is some internal cache that gets reused when the column name is the same.

http://plnkr.co/edit/kpn77emSbQLb4VbVe0XK?p=preview

@c0bra c0bra added this to the 3.1 milestone Jun 29, 2015
@c0bra
Copy link
Contributor

c0bra commented Jun 29, 2015

@lpand
Copy link
Contributor

lpand commented Jun 30, 2015

I believe this if-else chain is the problem: https://github.com/angular-ui/ng-grid/blob/master/src/js/core/factories/GridColumn.js#L431

if (gridUtil.isNullOrUndefined(self.width) || !angular.isNumber(self.width)) {
      if (gridUtil.isNullOrUndefined(colDef.width)) {
        self.width = '*';
      }

      [....] 

        // Is a number, use it as the width
        else {
          self.width = colDef.width;
        }
      }
    }

If width is defined and a number, it isn't updated. Setting widths as percentages makes the demo work.

@jmalkan
Copy link

jmalkan commented May 22, 2018

@mportuga this is closed. Are you planning to reopen since you marked it as "good first issue"?

@mportuga
Copy link
Member

nope, I re-read it and noticed it has already been addressed in the latest version.

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

No branches or pull requests

5 participants