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

The grid menu does not display properly when frozen columns are used with the grid menu #436

Closed
songjiqiang opened this issue Sep 17, 2019 · 13 comments · Fixed by #438 or #439
Closed

Comments

@songjiqiang
Copy link

songjiqiang commented Sep 17, 2019

When frozen column and grid menu are used together, the menu will be displayed in the left frozen column panel. This seems not normal. When I look at the source code, I find that it is caused by the following code logic
I only set frozenColumn to 0 because I only need to freeze the checkbox column
image
image

@ghiscoding
Copy link
Collaborator

ghiscoding commented Sep 17, 2019

Does it work if you change the source code to ... >= 0 ?

This might give issues on regular grid though (when not using frozenColumn), I'm not sure what is the default of the grid option frozenColumn, we have to check that too... EDIT: the default is -1 so it should be ok to change to >= 0 but I'd like to have confirmation that it works.

@songjiqiang
Copy link
Author

songjiqiang commented Sep 17, 2019

The default value of frozenColumn is -1. After I changed the source code in this way, the test was fine. Here is my unit test
image
image

@ghiscoding
Copy link
Collaborator

Have you tested with a regular grid (without any frozen)?

@songjiqiang
Copy link
Author

Yes, I didn't set any frozen columns and normal grids work fine. Here are the unit tests I did not set frozen columns
image

@ghiscoding
Copy link
Collaborator

Ok good, I'll do a fix later.. unless you want to create a PR (Pull Request) yourself? That would be nice.

Thanks

@songjiqiang
Copy link
Author

Ok, I will not create PR by myself for the time being. I will upgrade it after your official repair. At present, I temporarily solve this problem by changing the source code.
thank you

@songjiqiang
Copy link
Author

songjiqiang commented Sep 18, 2019

@ghiscoding If frozenColumn is set to 0, this judgment is problematic.For example, if(0) is actually false, then the judgment logic should be deleted, Please check the screenshot of my question, thank you
图片

@ghiscoding
Copy link
Collaborator

oh damn I didn't catch that since you didn't mention it earlier. I'll fix that later today.

@ghiscoding ghiscoding reopened this Sep 18, 2019
@songjiqiang
Copy link
Author

Ok, thank you. I took a screenshot of this question in the comments yesterday. Thank you
image

ghiscoding added a commit that referenced this issue Sep 18, 2019
- fixes the previous commit to fix the same issue, we should allow frozenColumns to be 0 or more and we should check that the frozenColumn property exist
@ghiscoding
Copy link
Collaborator

I raised another PR, removing frozenColumn is not ideal the property might not exist. What is best to use hasOwnProperty('frozenColumn') and that is what I changed in new PR.

@songjiqiang
Copy link
Author

songjiqiang commented Sep 18, 2019

OK, you can also solve it in the following way,
I feel that this approach is more rigorous, because it can be controlled to allow only numbers, and other types of values I think are ineffective.
&& $.type(_gridOptions.frozenColumn) === 'number'

@ghiscoding
Copy link
Collaborator

ghiscoding commented Sep 18, 2019

I already did another PR #439 and tested it locally

@songjiqiang
Copy link
Author

OK,thanks

@6pac 6pac closed this as completed in #439 Sep 18, 2019
6pac added a commit that referenced this issue Sep 18, 2019
fix(gridMenu): Menu should work with Frozen Col 0, fixes #436
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants