-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Update Expandable
style
#12457
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
Conversation
For this, I extracted the `Alert` component stuff into a more generic `Callout` component, which the `Alert` extends. The `Expandable` can now use the same basis, ensuring we have the same style etc, and just pass some more stuff into it. For this, the `Callout` component also allows to pass `id` and `titleOnClick` attributes which we need for the Expandable.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Bundle ReportChanges will increase total bundle size by 2.03kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
View changes by path for bundle: sentry-docs-server-cjs
|
} | ||
|
||
.callout-content { | ||
min-width: 0; |
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.
Without this, e.g. code boxes can lead to exceeding the width of the content. this should "fix" this.
src/components/callout/index.tsx
Outdated
} | ||
|
||
// We want to avoid actually triggering the link | ||
const wrappedOnClick = onClick |
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.
would move this into a useCallback
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 changed it, better like this?
src/components/expandable.tsx
Outdated
|
||
export function Expandable({title, children, permalink}: Props) { | ||
const [isExpanded, setIsExpanded] = useState(false); | ||
const defaultIsExpanded = id && window.location.hash === `#${id}`; |
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'll need to move this into an effect to avoid hydration errors
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.
moved this around!
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 neat! 💅
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 left a little feedback - but the PR itself seems fine. Thank you!
…zation-settings * master: (511 commits) docs(cloudflare): several semantic and stylistic fixes in code examples for cloudflare workers (#12482) chore(replay): Clarify network bandwidth and show how to reduce perf impact for mobile SDKs (#12479) chore(replay): Update product docs FAQ (#12474) ref: Drop remaining usage of `<Note>` (#12476) Update debugmeta.mdx (#12436) Fix queue_producer_transaction string termination error in example code (#12463) Adjust for sentry-ruby changes (#12455) Fixed incorrect instrument.mjs filename (#12454) Updating feature flags docs location (#12453) ref: Remove `<Note>` component in favor of using `<Alert>` (#12467) feat: send content feedback plausible events (#12400) docs(native): document how to set attribute on spans/transactions feat(express): Add warning about express v5 (#12465) chore(jira): Add instructions for installing Jira for self-hosted sentry users (#12433) chore: Update Codecov Bundler Plugin Version (#12459) Bump API schema to 421589ca (#12461) fix(platform): Enable scrolling via keyboard buttons (#12460) feat: Update `Expandable` style (#12457) Add Kotlin examples and fix typo in Java and Android Scopes docs (#12458) doc(js): Document how to set attribute on all spans (#12449) ...
For this, I extracted the
Alert
component stuff into a more genericCallout
component, which theAlert
extends.The
Expandable
can now use the same basis, ensuring we have the same style etc, and just pass some more stuff into it.For this, the
Callout
component also allows to passid
andtitleOnClick
attributes which we need for the Expandable.Before
After
Permalinks
I made sure that permalinks continue to work. I tweaked the behavior a bit, now it does not scroll into view when you open a permalink expandable, this should be a bit more subtle now. You can try the behavior on any Troubleshoot page in the JS repo, for example.