Feature/DYN 5296 notification center with dynamic height #36
Conversation
d2a8d9a to
24c2603
Compare
|
@Enzo707 One test failure, please check PR build check |
24c2603 to
ace10a1
Compare
|
@QilongTang @reddyashish please review this. Thanks |
tests/e2e/e2e.test.js
Outdated
| const header = await page.waitForSelector('#root > div > div > header'); | ||
| await expect(header).toMatch('Notifications'); | ||
| }); | ||
| // it('should have correct header', async () => { |
There was a problem hiding this comment.
Add a todo comment on why we are commenting this failing test. Otherwise, we can just remove it.
There was a problem hiding this comment.
Sure, I'll remove it!
| @@ -0,0 +1 @@ | |||
| export { default as EmptyStateArchiver } from './EmptyStateArchiver'; | |||
There was a problem hiding this comment.
sorry i might have missed but why do we need EmptyStateArchiver? Is that referenced from somewhere?
There was a problem hiding this comment.
This is just a good practice, it's recommended always have an index file that contains all the exports. So then, we could import components from the main folder. In this particular case imagine that we add more icons, we could use named imports from /icons instead of adding multiple ones.
This:
import { EmptyStateArchiver, Icon2, Icon3... } from './icons';
Instead of:
import EmptyStateArchiver from './icons/EmptyStateArchiver';
import Icon2 from './icons/icon2';
import ...
ace10a1 to
b445ab2
Compare
This PR Implements a mechanism to send any change related to notification center height to Dynamo so then we could trigger the notification popup resizing.