-
Notifications
You must be signed in to change notification settings - Fork 46
Adds new USP component formatting #3138
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
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
src/pages/docs/platform/index.mdx
Outdated
| meta_description: "An introduction to Ably and its highly-available, scalable platform." | ||
| --- | ||
|
|
||
| import { Banner } from '../../../components/blocks/banners' |
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 shouldn't be using it like this. Have a look at how If component is included in the MDXWrapper, that way you don't need the import at the top of every file that uses the Banner.
https://github.com/ably/docs/blob/main/src/components/Layout/MDXWrapper.tsx
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.
Cool.. I've added:
import { Banner } from '../blocks/banners';
to src/components/Layout/MDXWrapper.tsx
|
The test failed due to prettier: |
GregHolmes
left a comment
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.
Looks good! There are a couple of comments though.
The header of the Aside shouldn't be a header but instead slightly increased and boldened text.
The padding in the class of the Aside is too big compared to the design. I think once these changes are made it'll look clean on the page!
| )} | ||
| > | ||
| <div className="flex-1 mt-1.5 mb-0.25"> | ||
| {headline && <h3 className="ui-text-h3 font-bold text-neutral-1300 mb-3">{headline}</h3>} |
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.
I don't think this is meant to be a header. From what I recall it was the same size as the text (or ever so slightly bigger than normal text) and boldened. H3 makes it stick out way too much on the page.
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.
Font size of header text is 16 where normal text is 14.
| {...rest} | ||
| data-type={dataType} | ||
| className={cn( | ||
| 'border-l-[1.5px] px-6 pt-2.5 pb-1.5 my-6 rounded-r-lg w-full', |
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.
Padding top is currently 0.625rem, which equates to 10px. The design's one is 8px for example.
I think looking at the Figma designs it should be more: 'border-l-[1.5px] px-6 py-2 my-6 rounded-r-lg w-full gap-1',
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.
yeah impimented that and it looked way off
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.
I will re-do and see if shows any better but i did have to have a play around to make it look half decent
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.
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.
alot more space at the bottom
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.
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.
6e7fc72 to
7911567
Compare
7911567 to
273a4e6
Compare
273a4e6 to
f7c337b
Compare
f7c337b to
c09540d
Compare
- Add banner variant to Admonition component with headline support and reduced bottom padding (mb-0) - Add banner aside to platform index page with durability message - Banner uses orange left border (#FF5416) and transparent background - Special rendering logic for banner variant with headline and content sections 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
320afad to
b4e8302
Compare
8f50d36 to
cd0dc74
Compare
- Add Banner component with styling and MDX support - Restore evidence aside support in Admonition component - Update MDXWrapper to include Banner in MDX components - Fix Aside component to properly support evidence type with custom rendering 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>




Description
This PR adds the formatting to the VALs needed for the USB call outs. The USB call outs on other branches will be merged here.
I have added just one placeholder USP so that you can have a look. I will delete this in a future
-- fixuponce approved.Figma file: https://www.figma.com/design/HuzNhcZmPY9vnL6tVLOPXg/WIP-Docs-Components?node-id=733-3983&p=f
Checklist