Angular/out of context docs rendering stories #15002
Conversation
Nx Cloud ReportWe didn't find any information for the current pull request with the commit dc6cf48. Check the Getting started section to configure the app. Sent with 💌 from NxCloud. |
BREAKING CHANGE @angular/elements and @webcomponents/custom-elements are now required as dependencies for angular app
ThibaudAV
force-pushed
the
angular/out-of-context-docs-rendering-stories
branch
from
6818fb1 to
f28c88e
Compare
ThibaudAV
force-pushed
the
angular/out-of-context-docs-rendering-stories
branch
2 times, most recently
from
693da9b to
6e6ce8c
Compare
ThibaudAV
changed the base branch from
next
to
tech/out-of-context-docs-rendering-stories
|
Thank you so much @ThibaudAV 🙇🙇🙇 |
Rework the renderer to have 2 renderers : CanvasRenderer and DocsRenderer The RendererService becomes deprecated with the ElementRendererService and can be removed soon
ThibaudAV
force-pushed
the
angular/out-of-context-docs-rendering-stories
branch
from
6e6ce8c to
9dda9a5
Compare
addons/storyshots/storyshots-core/src/frameworks/angular/renderTree.ts
Outdated
Show resolved
Hide resolved
| { | ||
| storyFnAngular: currentStory, | ||
| parameters: story.parameters, | ||
| // TODO : To change with the story Id in v7. Currently keep with static id to avoid changes in snapshots |
There was a problem hiding this comment.
Should we add an issue and link as TODO?
In my experience "TODO:" always get lost without an issue (that is taggable als 'do in v7' or something)
There was a problem hiding this comment.
I don't really know what to do. I don't use storyshots enough. So for me there is no real issue
Is it better to have shapshot with storybook-wrapper main div or with story id 🤷♂️
I can also change to a simple comment
| }; | ||
|
|
||
| // platform must be init only if render is called at least once | ||
| let platformRef: PlatformRef; |
There was a problem hiding this comment.
Should this be _platformRef instead so devs won't tend to use this variable instead of getPlatform?
Highly opinionated code suggestion 😄 I don't mind if you leave it as is
To me _name always feels like "don't touch this, use something else"
There was a problem hiding this comment.
Yes, that's the idea don't touch this, use something else :p
I haven't found a clean way to make the new platformRef
but to avoid mistakes with storyshots the platformBrowserDynamic must be call only if the render is called at least once.
I thought of doing it in the factory to give it to all Renderer instance but it was more code, not much clearer so I hid it here 😈
But I take all your ideas :)
app/angular/src/client/preview/angular-beta/AbstractRenderer.ts
Outdated
Show resolved
Hide resolved
| "@angular/cli": "^11.2.0", | ||
| "@angular/compiler-cli": "^11.2.0", | ||
| "@angular/elements": "^11.2.0", | ||
| "@angular-devkit/build-angular": "~0.1102.13", |
There was a problem hiding this comment.
In dependencies, you have installed 11.2.14 and here you install 11.2.13
|
Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks! |
|
Closing this as the original PR has also been canceled. If you think there are useful things to be extracted from this PR, please feel free to create a new PR in the current setup. Thanks so much for your work on this! 🙏 |
Issue:
Angular part after this : #14911
What I did
TODO 🙈
Sorry about refactoring. 😈
But to many case to handle like navigate between canvas and docs view or some particularities on each one need to use some patten to try to have clean code and not many function intertwined.
I hope it's better
How to test
Yesnana