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

Add feature menuSelectionCallback to pull updatedValues onClick of a ListItem #113

Closed
wants to merge 1 commit into from

Conversation

jwmann
Copy link
Contributor

@jwmann jwmann commented Dec 1, 2017

Fixes #112

@codecov-io
Copy link

Codecov Report

Merging #113 into master will decrease coverage by 0.39%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #113     +/-   ##
=========================================
- Coverage   40.97%   40.57%   -0.4%     
=========================================
  Files           6        6             
  Lines         205      207      +2     
=========================================
  Hits           84       84             
- Misses        121      123      +2
Impacted Files Coverage Δ
src/types/index.js 33.33% <ø> (ø)
src/defaultProps.js 53.84% <ø> (ø)
src/SuperSelectField.js 38.21% <0%> (ø)
...terial-ui-superselectfield/src/SuperSelectField.js
...an/material-ui-superselectfield/src/types/index.js
...Sharlaan/material-ui-superselectfield/src/utils.js
.../material-ui-superselectfield/src/FloatingLabel.js
...ial-ui-superselectfield/src/SelectionsPresenter.js
...n/material-ui-superselectfield/src/defaultProps.js
src/utils.js 36% <0%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2782171...f146b4b. Read the comment docs.

@jwmann
Copy link
Contributor Author

jwmann commented Dec 4, 2017

I'm trying to write some tests for this but it seems almost impossible.

describe('menuSelectionCallback', () => {
  it('should return updatedValues', () => {
    const wrapper = shallowWithContext(<SuperSelectField showAutocompleteThreshold="always">{testChildren}</SuperSelectField>)
    wrapper.simulate('click')
    const firstChild = wrapper.find('ListItem').first()
    firstChild.simulate('click', {
      preventDefault: () => {
      },
    });
    expect( wrapper.instance().props.menuSelectionCallback ).toHaveBeenCalled()
  })
})

Keep getting an error
TypeError: Cannot read property 'focus' of undefined or undefined is not an object (evaluating '_this.root.focus')

I also just noticed #23
I didn't realize this was a hot button issue.
I do think setting the state based onClick rather than when the menu is closed is necessary.
Not only that, but the original docs were actually incorrect as it said the state was change onClick but that's not the case with onChange.

@jwmann
Copy link
Contributor Author

jwmann commented Dec 4, 2017

Unfortunately because of #105 I can't even run this on my project without published merges.
I'm open to discussing these changes as well as test coverage. Hopefully we can resolve this without breaking any backwards compatibility.

@Sharlaan
Copy link
Owner

solved with #119

@Sharlaan Sharlaan closed this Dec 26, 2017
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.

3 participants