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.

Sign up for GitHub

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

Jump to bottom

gridOptions are out of sync with grid.getOptions() #49

Closed
jmzagorski opened this issue Apr 18, 2018 · 1 comment · Fixed by #51
Closed

gridOptions are out of sync with grid.getOptions() #49

jmzagorski opened this issue Apr 18, 2018 · 1 comment · Fixed by #51
Labels
bug

Comments

@jmzagorski
Copy link
Collaborator

jmzagorski commented Apr 18, 2018 •
edited
Loading

I'm submitting a bug report
Maybe this is a bug or maybe you did it by design

  • Framework Version:
    current

Please tell us about your environment:

  • Operating System:
    All

  • Browser:
    all

  • Language:
    TypeScript

Current behavior:
GridEventService passes grid and gridOptions, but gridOptions are out of date.

Expected/desired behavior:
gridOptions are up to date

  • What is the expected behavior?

  • What is the motivation / use case for changing the behavior?
    I was using attachOnClick in the GridEventService and notice you are passing the grid and gridOptions. However, if a property on the gridOptions are changed after AureliaSlickgridCustomElement initializes using grid.getOptions().fooBar = "new property", grid.getOptions() will not match gridOptions because that gridOptions are the original options used in the custom element. Slickgrid uses $.extend{} to create a copy of the gridOptions which is why these are out of sync

I know you are on vacation so don't respond to this :-). There is no rush, I just wanted to submit it as an issue/question before I forgot.
@ghiscoding
Copy link
Owner

Would you be able to make a PR to address it?

jmzagorski added a commit that referenced this issue Apr 19, 2018
@jmzagorski
fix(eventService): use grid.getOptions for gridDefinition
e67e93a 
This fixes a potential problem where the grid options as a parameter in the method are different than grid.getOptions. Users are allowed to change options after the grid is created by calling, for example, `grid.getOptions().showHeaderValue = false`;. To make sure the options are up to date we need to call `grid.getOptions` when passing it to the events. Looking over `AureliaSlickgridCustomElement`, it should be fine to use `this.gridOptions` because most of the configuration is happening when the custom element is being created. The only potential issue I could see is if the `datasetChanged` fires after the grid is created. Then any call to `this.gridOptions` could potentially be out of data if the developer allows the user to change options later on. You can see this bug in example3 when you switch `autoEdit` off click the "edit" button. The gridDefinition has `autoEdit=true`

closes #49
@jmzagorski jmzagorski mentioned this issue Apr 19, 2018
Merged
@ghiscoding ghiscoding mentioned this issue Apr 30, 2018
Merged
@ghiscoding ghiscoding added the bug label Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(eventService): use grid.getOptions for gridDefinition ghiscoding/aurelia-slickgrid
2 participants
@ghiscoding @jmzagorski