Skip to content

Fix Issue 556#561

Merged
malonecj merged 6 commits intomasterfrom
dm-fix-filter-header-bug
Jan 6, 2017
Merged

Fix Issue 556#561
malonecj merged 6 commits intomasterfrom
dm-fix-filter-header-bug

Conversation

@diegomurakami
Copy link
Copy Markdown
Contributor

@diegomurakami diegomurakami commented Dec 8, 2016

Description

Fix #556

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
New checkFocus method was implemented recently on Cell component. This method checks the selected cell when filters are selected and used. The problem is that [0,0] is selected when filters are used, causing [0,0] to be focused after filter change.

What is the new behavior?
When the user clicks on the Header, we call a new method onHeaderClick which sets [-1,-1] as the current selection of the Grid using onCellClick from cellMetaData (injected as a new prop to the Header component).

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[x] No

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 90fe95c on dm-fix-filter-header-bug into ** on master**.

Comment thread src/Header.js
},

// Set the cell selection to -1 x -1 when clicking on the header
onHeaderClick() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diegomurakami please add a test for this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@malonecj sorry for the delay. It's done.

@kbergin
Copy link
Copy Markdown

kbergin commented Jan 3, 2017

@diegomurakami Thank you for fixing this! Looking forward to the merge.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling f568450 on dm-fix-filter-header-bug into ** on master**.

const headerDiv = wrapper.find('div');
headerDiv.simulate('click');
expect(testAllProps.cellMetaData.onCellClick).toHaveBeenCalled();
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be verifying the parameters of cellMetaData. Its important to verify that the selected idx and rowIdx is -1,-1

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling c46bc98 on dm-fix-filter-header-bug into ** on master**.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 4c52568 on dm-fix-filter-header-bug into ** on master**.

@malonecj malonecj merged commit 20c478c into master Jan 6, 2017
@diegomurakami
Copy link
Copy Markdown
Contributor Author

@malonecj haha thanks !

@diegomurakami diegomurakami deleted the dm-fix-filter-header-bug branch March 16, 2017 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filter text box loses focus while user is typing

5 participants