Add S4A communication for anchor headers #2982
Conversation
ghost
self-requested a review
ghost
mentioned this pull request
kmcgrady
left a comment
kmcgrady
left a comment
There was a problem hiding this comment.
This looks good and probably could benefit from unit tests.
As a side note: I'm concerned about how we inject s4a communication across our system. It's fine in one place, but it's becoming a polluted landscape. Might not want to resolve now, but it might be good to talk about the longer issue in the end.
|
|
||
| React.useEffect(() => { | ||
| const listener = (): void => { | ||
| s4aCommunication.sendMessage({ |
There was a problem hiding this comment.
I think if we add the hoc here, we are going to initiate the 'ready' procedure twice, I would just import sendS4AMessage as we need it only for that.
adding to @kmcgrady's comment, We could add the event listener directly into the comm HOC unless there is some constraint that we need it into the ReportView component? |
|
Yeah, that's a better idea than what I'm doing. I felt awkward putting it in |
* Add anchors to Markdown headers (#2513) * save progress * save work * remove junk code * fix lint errors * clean up code a bit * fix incorrect type * remove debug code * fix heading numbers * make requested changes and clean up HeadingWithAnchor * only scroll once * fix issues * fix jslint * clean things up * handle edge case * missing a void * fix failing test * add e2e tests * add e2e tests * clean up code by using _.once() * fix lint * fix dynamic scrolling * fix typo * make requested changes * run formatting and remove unused import * remove another unused import * make requested changes * add nice link icon to left of headers * make requested design changes (#2726) * Fix anchor styling to work with dark mode (#3035) * fix anchor styling * Revert "fix anchor styling" This reverts commit cb5d7fd. * fix failing cypress tests * fix styling * Add S4A communication for anchor headers (#2982) * add communication with s4a * fix bug * save work * save work * remove console * add tests * remove useless import * fix failing tests * Remove Markdown anchors from sidebar (#3059) * remove markdown anchors from sidebar * fix test * add tests * Fix failing Cypress test for Markdown Anchors (#3077) * see if this fixes things * fix things * fix * fix scrolling * add spacer * save work * remove unneeded changes
* Add anchors to Markdown headers (#2513) * save progress * save work * remove junk code * fix lint errors * clean up code a bit * fix incorrect type * remove debug code * fix heading numbers * make requested changes and clean up HeadingWithAnchor * only scroll once * fix issues * fix jslint * clean things up * handle edge case * missing a void * fix failing test * add e2e tests * add e2e tests * clean up code by using _.once() * fix lint * fix dynamic scrolling * fix typo * make requested changes * run formatting and remove unused import * remove another unused import * make requested changes * add nice link icon to left of headers * make requested design changes (#2726) * Fix anchor styling to work with dark mode (#3035) * fix anchor styling * Revert "fix anchor styling" This reverts commit cb5d7fd. * fix failing cypress tests * fix styling * Add S4A communication for anchor headers (#2982) * add communication with s4a * fix bug * save work * save work * remove console * add tests * remove useless import * fix failing tests * Remove Markdown anchors from sidebar (#3059) * remove markdown anchors from sidebar * fix test * add tests * Fix failing Cypress test for Markdown Anchors (#3077) * see if this fixes things * fix things * fix * fix scrolling * add spacer * save work * remove unneeded changes
2 participants
Core's counterpart to https://github.com/streamlit/s4t/pull/1034. For Markdown Anchors to work, core and cloud need to notify each other when the browser URL hash changes. This PR
hashchangehandler that sends anUPDATE_HASHmessage to CloudUPDATE_HASHmessages from Cloud and updates our own hash