-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Allow CatalogEntityDetails to be opened anywhere #6939
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
Conversation
bdbfaf2
to
4364c9c
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
4364c9c
to
46265dd
Compare
Conflicts have been resolved. A maintainer will review the pull request shortly. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
46265dd
to
6551d7a
Compare
Conflicts have been resolved. A maintainer will review the pull request shortly. |
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.
doing some testing
@@ -0,0 +1,19 @@ | |||
/** |
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.
injectable creep? So many injectables
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: Sebastian Malton <[email protected]>
Signed-off-by: Sebastian Malton <[email protected]>
Signed-off-by: Sebastian Malton <[email protected]>
Signed-off-by: Sebastian Malton <[email protected]>
Signed-off-by: Sebastian Malton <[email protected]>
Signed-off-by: Sebastian Malton <[email protected]>
ec0fea5
to
9103d4d
Compare
Conflicts have been resolved. A maintainer will review the pull request shortly. |
@@ -129,6 +133,7 @@ describe("opening catalog entity details panel", () => { | |||
describe("when opening the menu 'some-kubernetes-cluster'", () => { | |||
beforeEach(() => { | |||
rendered.getByTestId("icon-for-menu-actions-for-catalog-for-some-entity-id").click(); | |||
advanceFakeTime(1000); |
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.
Is the 1000ms something special here? The name of describe doesn't really provide any information and I would have to dig deeper in the code. What happens after 999ms?
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.
This is to cover animations
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.
Advancing of the fake time is not clear in the behavioural tests. I would like to see what happens after e.g. 999ms and 1000ms so that I can immediately see what's the time were waiting for. Also it would start bringing up more corner cases, e.g. what happens if I click something twice which does something only after 1000ms.
Signed-off-by: Sebastian Malton [email protected]
Also expose this functionality to the extension API.
resolves #4963