-
Notifications
You must be signed in to change notification settings - Fork 2
dismiss editcard pressing Escape #2464
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
@Dantemss any idea why ci:test:ci:build is failling? As far as I kno, I haven't touched anything related with the timeout is showing |
@jomcarvajal it could be something is throwing an error on the page or something like that. See if you can get the test to open in a browser in debug mode, or take a screenshot and see what is being rendered. |
@Dantemss seems like prevent default behavior of Enter inside CardWrapper was causing problems with SearchResultsSidebar. Should be working now! In any case, could be great to review comments from card to have a better idea of this change. |
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 apparently had a comment here that was pending
export const editCardVisibilityHandler = (state: Map<string, boolean>, action: { type: string; id?: string }) => { | ||
switch (action.type) { | ||
case 'SHOW': { | ||
if (!action.id) return state; |
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 does nothing without an action.id
right? Maybe it should throw?
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.
The thing is editCardVisibilityHandler will be part of CardWrapper so they are other interactive elements without action.id that can enter inot this code so throwing it would cause more troubles. That's why I just returned the state
CORE-806