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

Fix suggestions not disappering under some conditions #566

Merged
merged 5 commits into from
Sep 10, 2018
Merged

Fix suggestions not disappering under some conditions #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
15a8cab
Fix suggestions not disappering under some conditions
aberezkin Sep 7, 2018
ce65204
Focus input field on touch move
aberezkin Sep 10, 2018
c0533c3
Add tests
aberezkin Sep 10, 2018
e734159
Fix typo
aberezkin Sep 10, 2018
3453851
Add test for input blur after drag
aberezkin Sep 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 40 additions & 5 deletions src/Autosuggest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,13 @@ export default class Autosuggest extends Component {

this.justPressedUpDown = false;
this.justMouseEntered = false;

this.pressedSuggestion = null;
}

componentDidMount() {
document.addEventListener('mousedown', this.onDocumentMouseDown);
document.addEventListener('mouseup', this.onDocumentMouseUp);

this.input = this.autowhatever.input;
this.suggestionsContainer = this.autowhatever.itemsContainer;
Expand Down Expand Up @@ -169,6 +172,7 @@ export default class Autosuggest extends Component {

componentWillUnmount() {
document.removeEventListener('mousedown', this.onDocumentMouseDown);
document.removeEventListener('mouseup', this.onDocumentMouseUp);
}

updateHighlightedSuggestion(sectionIndex, suggestionIndex, prevValue) {
Expand Down Expand Up @@ -332,6 +336,9 @@ export default class Autosuggest extends Component {

onSuggestionMouseEnter = (event, { sectionIndex, itemIndex }) => {
this.updateHighlightedSuggestion(sectionIndex, itemIndex);
if (event.target === this.pressedSuggestion) {
this.justSelectedSuggestion = true;
}

this.justMouseEntered = true;

Expand All @@ -344,8 +351,20 @@ export default class Autosuggest extends Component {
this.updateHighlightedSuggestion(this.props.multiSection ? 0 : null, 0);
};

onSuggestionMouseDown = () => {
this.justSelectedSuggestion = true;
onDocumentMouseUp = () => {
if (this.pressedSuggestion && !this.justSelectedSuggestion) {
this.pressedSuggestion = null;
this.input.focus();
}
};

onSuggestionMouseDown = e => {
// Checking if this.justSelectedSuggestion is already true to not duplicate touch events in chrome
// See: https://github.com/facebook/react/issues/9809#issuecomment-413978405
if (!this.justSelectedSuggestion) {
this.justSelectedSuggestion = true;
this.pressedSuggestion = e.target;
}
};

onSuggestionsClearRequested = () => {
Expand Down Expand Up @@ -427,18 +446,34 @@ export default class Autosuggest extends Component {
onBlur && onBlur(this.blurEvent, { highlightedSuggestion });
};

resetHighlightedSuggestionOnMouseLeave = () => {
onSuggestionMouseLeave = e => {
this.resetHighlightedSuggestion(false); // shouldResetValueBeforeUpDown
if (this.justSelectedSuggestion && e.target === this.pressedSuggestion) {
this.justSelectedSuggestion = false;
}
};

onSuggestionTouchStart = () => {
this.justSelectedSuggestion = true;
// todo: event.preventDefault when https://github.com/facebook/react/issues/2043
// todo: gets released so onSuggestionMouseDown won't fire in chrome
};

onSuggestionTouchMove = () => {
this.justSelectedSuggestion = false;
this.pressedSuggestion = null;
this.input.focus();
Copy link

Choose a reason for hiding this comment

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

Why would you need to refocus the input when onTouchMove?

Copy link
Collaborator Author

@aberezkin aberezkin Mar 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Honestly, I don't remember what exactly happened. But probably touchMove did remove focus from input and that caused suggestions to not disappear on touching something other than input.

Copy link

Choose a reason for hiding this comment

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

Thanks, I ended up extending the library myself to allow suggestions to remain on screen while dismissing keyboard on an iOS/Android device. So far I haven't had any complications due to rewriting this method. Would you be open to a PR changing this?

Copy link
Collaborator Author

@aberezkin aberezkin Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Of course I will be open to any PR but I am just a contributor. I think it's to better ask @moroshko about this. Anyway there are tests to prevent degradation in this area. Also, I think it would be reasonable to add a prop to control behaviour introduced by your PR, something like endureKeyboardDismiss={false|true}.

Copy link

Choose a reason for hiding this comment

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

Yes exactly!! Cool I will throw something together this week.

};

itemProps = ({ sectionIndex, itemIndex }) => {
return {
'data-section-index': sectionIndex,
'data-suggestion-index': itemIndex,
onMouseEnter: this.onSuggestionMouseEnter,
onMouseLeave: this.resetHighlightedSuggestionOnMouseLeave,
onMouseLeave: this.onSuggestionMouseLeave,
onMouseDown: this.onSuggestionMouseDown,
onTouchStart: this.onSuggestionMouseDown, // Because on iOS `onMouseDown` is not triggered
onTouchStart: this.onSuggestionTouchStart,
onTouchMove: this.onSuggestionTouchMove,
onClick: this.onSuggestionClick
};
};
Expand Down
19 changes: 9 additions & 10 deletions test/focus-first-suggestion/AutosuggestApp.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,15 @@ describe('Autosuggest with highlightFirstSuggestion={true}', () => {
onSuggestionSelected.reset();
clickEnter();
expect(onSuggestionSelected).to.have.been.calledOnce;
expect(onSuggestionSelected).to.have.been.calledWithExactly(
syntheticEventMatcher,
{
suggestion: { name: 'Perl', year: 1987 },
suggestionValue: 'Perl',
suggestionIndex: 0,
sectionIndex: null,
method: 'enter'
}
);
expect(
onSuggestionSelected
).to.have.been.calledWithExactly(syntheticEventMatcher, {
suggestion: { name: 'Perl', year: 1987 },
suggestionValue: 'Perl',
suggestionIndex: 0,
sectionIndex: null,
method: 'enter'
});
});
});

Expand Down
71 changes: 64 additions & 7 deletions test/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,11 @@ export const getSuggestion = suggestionIndex => {
throw Error(
`
Cannot find suggestion #${suggestionIndex}.
${
suggestions.length === 0
? 'No suggestions found.'
: `Only ${suggestions.length} suggestion${
suggestions.length === 1 ? '' : 's'
} found.`
}
${suggestions.length === 0
? 'No suggestions found.'
: `Only ${suggestions.length} suggestion${suggestions.length === 1
? ''
: 's'} found.`}
`
);
}
Expand Down Expand Up @@ -181,6 +179,30 @@ const mouseDownDocument = target => {
);
};

export const mouseUpDocument = target => {
document.dispatchEvent(
new window.CustomEvent(
'mouseup',
target
? {
detail: {
// must be 'detail' accoring to docs: https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Creating_and_triggering_events#Adding_custom_data_–_CustomEvent()
target
}
}
: null
)
);
};

const touchStartSuggestion = suggestionIndex => {
Simulate.touchStart(getSuggestion(suggestionIndex));
};

const touchMoveSuggestion = suggestionIndex => {
Simulate.touchMove(getSuggestion(suggestionIndex));
};

// It doesn't feel right to emulate all the DOM events by copying the implementation.
// Please show me a better way to emulate this.
export const clickSuggestion = suggestionIndex => {
Expand All @@ -195,6 +217,41 @@ export const clickSuggestion = suggestionIndex => {
clock.tick(1);
};

// Simulates only mouse events since on touch devices dragging considered as a scroll and is a different case.
export const dragSuggestionOut = suggestionIndex => {
const suggestion = getSuggestion(suggestionIndex);

mouseEnterSuggestion(suggestionIndex);
mouseDownDocument(suggestion);
mouseDownSuggestion(suggestionIndex);
mouseLeaveSuggestion(suggestionIndex);
mouseUpDocument();
};

export const dragSuggestionOutAndIn = suggestionIndex => {
const suggestion = getSuggestion(suggestionIndex);

mouseEnterSuggestion(suggestionIndex);
mouseDownDocument(suggestion);
mouseDownSuggestion(suggestionIndex);
mouseLeaveSuggestion(suggestionIndex);
mouseEnterSuggestion(suggestionIndex);
mouseUpDocument();
blurInput();
focusInput();
Simulate.click(suggestion);
clock.tick(1);
};

// Simulates mouse events as well as touch events since some browsers (chrome) mirror them and we should handle this.
// Order of events is implemented according to docs: https://developer.mozilla.org/en-US/docs/Web/API/Touch_events/Supporting_both_TouchEvent_and_MouseEvent
export const dragSuggestionOutTouch = suggestionIndex => {
touchStartSuggestion(suggestionIndex);
touchMoveSuggestion(suggestionIndex);
mouseDownSuggestion(suggestionIndex);
mouseUpDocument();
};

export const clickSuggestionsContainer = () => {
mouseDownDocument(suggestionsContainer);
blurInput();
Expand Down
79 changes: 58 additions & 21 deletions test/plain-list/AutosuggestApp.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {
mouseLeaveSuggestion,
clickSuggestion,
clickSuggestionsContainer,
dragSuggestionOut,
dragSuggestionOutTouch,
focusInput,
blurInput,
clickEscape,
Expand All @@ -28,7 +30,9 @@ import {
focusAndSetInputValue,
isInputFocused,
clearEvents,
getEvents
getEvents,
mouseUpDocument,
dragSuggestionOutAndIn
} from '../helpers';
import AutosuggestApp, {
getSuggestionValue,
Expand Down Expand Up @@ -316,6 +320,41 @@ describe('Default Autosuggest', () => {
});
});

// Tests for these bugs
// https://github.com/moroshko/react-autosuggest/issues/412#issuecomment-318627754
// https://github.com/moroshko/react-autosuggest/issues/380
describe('when suggestion is dragged', () => {
beforeEach(() => {
focusAndSetInputValue('p');
});

it('should keep the focus on input when suggestion is dragged', () => {
dragSuggestionOut(1);
expect(isInputFocused()).to.equal(true);
});

it('should clear suggestions if input blurred after suggestion drag', () => {
dragSuggestionOut(1);
blurInput();
expectSuggestions([]);
});

it('should keep the focus on input when suggestion is dragged on touch devices', () => {
dragSuggestionOutTouch(1);
expect(isInputFocused()).to.equal(true);
});

it("should select a suggestion if it's dragged and mouse enters back", () => {
dragSuggestionOutAndIn(1);
expectInputValue('PHP');
});

it('should not focus input on document mouse up', () => {
mouseUpDocument();
expect(isInputFocused()).to.equal(false);
});
});

describe('getSuggestionValue', () => {
beforeEach(() => {
getSuggestionValue.reset();
Expand Down Expand Up @@ -563,32 +602,30 @@ describe('Default Autosuggest', () => {
it('should be called once with the right parameters when suggestion is clicked', () => {
clickSuggestion(1);
expect(onSuggestionSelected).to.have.been.calledOnce;
expect(onSuggestionSelected).to.have.been.calledWithExactly(
syntheticEventMatcher,
{
suggestion: { name: 'JavaScript', year: 1995 },
suggestionValue: 'JavaScript',
suggestionIndex: 1,
sectionIndex: null,
method: 'click'
}
);
expect(
onSuggestionSelected
).to.have.been.calledWithExactly(syntheticEventMatcher, {
suggestion: { name: 'JavaScript', year: 1995 },
suggestionValue: 'JavaScript',
suggestionIndex: 1,
sectionIndex: null,
method: 'click'
});
});

it('should be called once with the right parameters when Enter is pressed and suggestion is highlighted', () => {
clickDown();
clickEnter();
expect(onSuggestionSelected).to.have.been.calledOnce;
expect(onSuggestionSelected).to.have.been.calledWithExactly(
syntheticEventMatcher,
{
suggestion: { name: 'Java', year: 1995 },
suggestionValue: 'Java',
suggestionIndex: 0,
sectionIndex: null,
method: 'enter'
}
);
expect(
onSuggestionSelected
).to.have.been.calledWithExactly(syntheticEventMatcher, {
suggestion: { name: 'Java', year: 1995 },
suggestionValue: 'Java',
suggestionIndex: 0,
sectionIndex: null,
method: 'enter'
});
});

it('should not be called when Enter is pressed and there is no highlighted suggestion', () => {
Expand Down