-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Render navigation and addons panels even when they are hidden #2336
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2336 +/- ##
=========================================
- Coverage 21.23% 21% -0.23%
=========================================
Files 283 283
Lines 6155 6150 -5
Branches 727 732 +5
=========================================
- Hits 1307 1292 -15
- Misses 4298 4302 +4
- Partials 550 556 +6
Continue to review full report at Codecov.
|
I love this! We have currently the issue with it with some in house addons! If you need some help/feedback do not hesitate 😄 |
expect(markup).toMatch(/LeftPanel/); | ||
expect(markup).toMatch(/DownPanel/); | ||
expect(markup).toMatch(/Preview/); | ||
expect(wrap).toMatchSnapshot(); |
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 snapshot is huge, is there some sane way to deal with that, or is it inevitable?
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.
We can explicitly test the presence of particular styles instead
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.
Did that
expect(markup).not.toMatch(/LeftPanel/); | ||
expect(markup).not.toMatch(/DownPanel/); | ||
expect(markup).toMatch(/Preview/); | ||
expect(wrap).toMatchSnapshot(); |
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.
same
expect(markup).not.toMatch(/LeftPanel/); | ||
expect(markup).toMatch(/DownPanel/); | ||
expect(markup).toMatch(/Preview/); | ||
expect(wrap).toMatchSnapshot(); |
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.
same
expect(markup).toMatch(/LeftPanel/); | ||
expect(markup).not.toMatch(/DownPanel/); | ||
expect(markup).toMatch(/Preview/); | ||
expect(wrap).toMatchSnapshot(); |
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.
same
Issue: #2280
Some addons (backgrounds, notes, probably others) send data from preview to addon panel when the former is rendered. The problem is that if the panel is hidden at that point, it's not actually rendered, so it will never receive that data.
What I did
Render addon panel with
display: none
when it should be hidden. Do the same with navigation panel just for consistency.How to test
Open backgrounds and notes examples in cra-kitchen-sink, toggle addon panel with
⌘ ⇧ D