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

fix(components/typeahead): use a flag for the display mode #1101

Conversation

ncomont
Copy link
Contributor

@ncomont ncomont commented Feb 26, 2018

What is the problem this PR is trying to solve?

The only way to display the typeahead input when using the toggle functionnality is to delete the onToggle property, which is obviously a bad thing.

What is the chosen solution to this problem?

Add a docked property.

Please check if the PR fulfills these requirements

  • The PR commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Related design / discussions / pages (not in jira), if any, are all linked or available in the PR

[x] This PR introduces a breaking change

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

function Typeahead({ onToggle, icon, position, ...rest }) {
if (onToggle) {
function Typeahead({ onToggle, icon, position, docked, ...rest }) {
if (docked === true && onToggle) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (!!docked && onToggle)

const wrapper = renderer
.create(<Typeahead {...props} />)
.toJSON();
const wrapper = renderer.create(<Typeahead {...props} />).toJSON();
Copy link
Contributor

Choose a reason for hiding this comment

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

where are the snap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why would you want to see it modified by this test ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Dunno, right.

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

const wrapper = renderer
.create(<Typeahead {...props} />)
.toJSON();
const wrapper = renderer.create(<Typeahead {...props} />).toJSON();
Copy link
Contributor

Choose a reason for hiding this comment

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

Dunno, right.

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

@build-travis-ci
Copy link
Collaborator

:octocat: Demo is available here

@jsomsanith-tlnd jsomsanith-tlnd merged commit ecf1c31 into master Feb 28, 2018
@jsomsanith-tlnd jsomsanith-tlnd deleted the ncomont/fix/components/typeahead/use_a_flag_for_the_display_mode branch February 28, 2018 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants