nteract / semiotic-docs

Docs for using Semiotic
https://semiotic.nteract.io/
32 stars 12 forks source link

[bug] Waterfall Chart Example Breaks #19

Closed hydrosquall closed 4 years ago

hydrosquall commented 4 years ago

The following error pops up in the console and the screen blanks out when navigating to this page using the build from the current master branch - I am guessing that this example might not be compatible with the current version (1.20.2) of Semiotic in some way.

hydrosquall commented 4 years ago

I downgraded Semiotic from 1.20.2 to 1.18.0, and the example worked again. Going to 1.19.11 led to the same console error as with 1.20+. As a result, I think something about the OrdinalFrame API changed in the between.

After doing some binary search, the break happened in the jump between 1.19.6 and 1.19.7, I might look more closely at this diff another time: https://github.com/nteract/semiotic/compare/v1.19.6...v1.19.7

emeeks commented 4 years ago

Thanks for taking a look into this. I believe there's an updated version that works but just hasn't been deployed.

hydrosquall commented 4 years ago

Ah, great! My concern had been that if we deployed the current “master” branch to the public URL, this example would stop working. However, if there’s already a fix on another branch somewhere, then we probably want to merge that before redeploying the site.

On Sun, Oct 6, 2019 at 2:27 PM Elijah Meeks notifications@github.com wrote:

Thanks for taking a look into this. I believe there's an updated version that works but just hasn't been deployed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nteract/semiotic-docs/issues/19?email_source=notifications&email_token=ACE2MM26OVOGKGNBKQABF4TQNIUYBA5CNFSM4I5ZWRA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAOQUUQ#issuecomment-538774098, or mute the thread https://github.com/notifications/unsubscribe-auth/ACE2MM6HCMJMXZ6V55XN363QNIUYBANCNFSM4I5ZWRAQ .

hydrosquall commented 4 years ago

Debugged a bit and I think this is what changed

https://github.com/nteract/semiotic/blob/4587fdd5c8424b0d0046e093c507b5431c7470a3/src/components/svg/pieceLayouts.tsx#L771-L779

https://github.com/nteract/semiotic-docs/blob/682b3936a01705349187e3909c1ed537fd5c23e6/src/examples/WaterfallChart.js#L209-L215

hydrosquall commented 4 years ago

Update

Bumping the version of semiotic in the docs to 1.20.3 the example, PR incoming.