-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[select] common test suite for all components #2646
Conversation
add Omnibar tests!Preview: documentation | landing | table |
😢 this suite relies on some of the new features i built for controlled usage. might have to wait till after that merges... |
fix common tests. revert this commit to bring back controlled tests.Preview: documentation | landing | table |
Omnibar disable usePortal in testsPreview: documentation | landing | table |
@@ -41,6 +41,9 @@ export interface IOmnibarProps<T> extends IListItemsProps<T> { | |||
/** Props to spread to `Overlay`. */ | |||
overlayProps?: Partial<IOverlayProps>; | |||
|
|||
/** The query string. */ | |||
query?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be initialQuery
given that changes are not respected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm planning to change this immediately after merge with the whole un/controlled feature, so let's keep it to this name so it's not "breaking"
@@ -159,6 +159,7 @@ export class Suggest<T> extends React.PureComponent<ISuggestProps<T>, ISuggestSt | |||
|
|||
this.selectText(); | |||
|
|||
// TODO Popover.openOnTargetFocus? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+?
onActiveItemChange: sinon.spy(), | ||
onItemSelect: sinon.spy(), | ||
renderer: sinon.spy(() => <div />), // must render something, | ||
renderer: sinon.spy((props: IQueryListRendererProps<IFilm>) => <div>{props.itemList}</div>), // must render something, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for this comment now
assert.isFalse(wrapper.state().isOpen, "popover should not open on focus"); | ||
// TODO fix later | ||
// simulateFocus(wrapper); | ||
// assert.isFalse(wrapper.state().isOpen, "popover should not open on focus"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when is later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments. Also echo @invliD's comments
@@ -159,6 +159,7 @@ export class Suggest<T> extends React.PureComponent<ISuggestProps<T>, ISuggestSt | |||
|
|||
this.selectText(); | |||
|
|||
// TODO Popover.openOnTargetFocus? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+?
@@ -53,7 +64,7 @@ describe("Suggest", () => { | |||
runEscTabKeyDownTests(Keys.TAB); | |||
}); | |||
|
|||
it("does not open popover on BACKSPACE, ARROW_LEFT, or ARROW_RIGHT", () => { | |||
xit("does not open popover on BACKSPACE, ARROW_LEFT, or ARROW_RIGHT", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this now disabled? Leave comment at minimum
commentsPreview: documentation | landing | table |
also add tests for Omnibar!