feat: adding first version of embeddable docs #5513
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for novu-design ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Here is the backend service for the docs as well https://github.com/novuhq/cloud-doc |
There was a problem hiding this comment.
Thank you for your hard work on this, David! The loom videos were super helpful for seeing the whole masterpiece come together and I think this feature will be very helpful for users.
I do have some large-scale feedback that I'm happy to discuss further; part of it may require input from Nik and Dima.
- As mentioned in
AppLayout, I'm concerned that the context wrapping the whole app could have significant performance implications. I'd like to propose an alternative structure that avoids touching the*Page.tsxfiles. Given that all of our documentation is tied to a specific page, we should be able to leverage theROUTESas a key for a look-up in a configuration file. This would allow us to correlate a route with a documentation path and any other static fields. Furthermore, it provides type-safety and centralizes the related content to minimize touchpoints. We could use a hook to match the current route to a config, and then render the button accordingly. - I strongly propose that we do not put the documentation in the header nav. I don't believe it's a typical pattern, and that bar is already fairly crowded (and may get more so if we choose to add Org and Env switchers there). Instead, I'd propose a FAB (Floating Action Button). They offer more flexibility and extensibility, feel more directly related to the content on the existing page, and seem to match common practices more. I did a quick look, and saw that Linear is using one for the same purpose.
Please let me know if you have any questions about these things or the other feedback in the PR, and again, nice work!
I have now fixed according to your feedback on 1. Number 2 would I love to get @scopsy thoughts on :) |
|
100% agree with @antonjoel82 on the top navigation bar point. I have already mentioned this earlier this week to nikolay here: https://www.figma.com/design/ybKRCB2ZO5DqICxjxzZGnW?node-id=701-74044#804551101 I would actually go further and say, that the inline documentation should be, well inline 😄 When it's located at the top navbar I would expect it will just open the global docs page for me, as the top navbar is the "global" scope for me personally. Here are a couple of potentially solutions we can iterate over:
Putting this note assigned, I really like how the modal itself and the docs look like! 😍 |
|
@scopsy @antonjoel82 @davidsoderberg Since we're only testing the functionality --- although the FAB idea is cool and I agree in the long run it would give us more flexibility as a small context menu --- for version one, I'd go with the "icon next to the page title" approach that @scopsy suggested. Thanks! |
|
It is now moved to the page title instead 🥳 
|
| <Alert | ||
| className={css({ | ||
| borderRadius: '75', | ||
| backgroundColor: 'mauve.60.dark !important', |
There was a problem hiding this comment.
All !important I have added in latest commit was needed to affect style of their components
There was a problem hiding this comment.
🤓 nitpick (non-blocking): While using the "new" base color tokens is better than using the legacy.* tokens, we should still be aiming to use semantic tokens as much as possible in application code so that we can better handle dark/light and in a semantic way.
| <Alert | ||
| className={css({ | ||
| borderRadius: '75', | ||
| backgroundColor: 'mauve.60.dark !important', |
There was a problem hiding this comment.
🤓 nitpick (non-blocking): While using the "new" base color tokens is better than using the legacy.* tokens, we should still be aiming to use semantic tokens as much as possible in application code so that we can better handle dark/light and in a semantic way.
antonjoel82
force-pushed
the
nv-3696-embeddable-documentation
branch
from
754fd7e
to
30b827d
Compare
|
@davidsoderberg I rebased your PR after merging the new Design System package and updated the imports for Panda so that everything should just work the same! |
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Your PR title is: Details: No scope found in pull request title "feat: adding first version of embeddable docs". Scope must match one of: root, api, inbound-mail, web, webhook, widget, worker, ws, ee-auth, ee-billing, ee-billing-web, ee-echo-api, ee-echo-web, ee-echo-worker, ee-dal, ee-shared-services, ee-translation, ee-translation-web, automation, dal, design-system, embed, novui, shared, shared-web, testing, application-generic, novu, novu-labs, client, echo, headless, nest, node, notification-center, angular-workspace, notification-center-angular, notification-center-vue, providers, stateless. |
https://www.loom.com/share/ed59a7cc9b834426811994be310e81c6?sid=03dfc7a1-13cf-42c1-8c03-7a31efcf7bae