Skip to content

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

Merged
merged 5 commits into from
Jan 24, 2025
Merged

feat: Update Expandable style #12457

merged 5 commits into from
Jan 24, 2025

Conversation

mydea
Copy link
Member

@mydea mydea commented Jan 24, 2025

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.

Before

Screenshot 2025-01-24 at 11 19 02
Screenshot 2025-01-24 at 11 19 07

After

Screenshot 2025-01-24 at 11 19 41

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.

mydea added 2 commits January 24, 2025 10:23
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.
Copy link

vercel bot commented Jan 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
develop-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2025 1:15pm
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2025 1:15pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
changelog ⬜️ Ignored (Inspect) Visit Preview Jan 24, 2025 1:15pm

Copy link

codecov bot commented Jan 24, 2025

Bundle Report

Changes will increase total bundle size by 2.03kB (0.01%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sentry-docs-server-cjs 10.61MB 1.23kB (0.01%) ⬆️
sentry-docs-client-array-push 9.3MB 800 bytes (0.01%) ⬆️
View changes by path for bundle: sentry-docs-server-cjs
File path Size Change
/platform-redirect 30.75kB 1 bytes (0.0%)
/[[...path]] 514.5kB 75 bytes (-0.01%)

}

.callout-content {
min-width: 0;
Copy link
Member Author

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.

}

// We want to avoid actually triggering the link
const wrappedOnClick = onClick
Copy link
Member

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

Copy link
Member Author

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?


export function Expandable({title, children, permalink}: Props) {
const [isExpanded, setIsExpanded] = useState(false);
const defaultIsExpanded = id && window.location.hash === `#${id}`;
Copy link
Member

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this around!

Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks neat! 💅

@stephanie-anderson
Copy link
Contributor

Thanks for taking care of this, @mydea! Could you have a look at the case, when there is special formatting involved, like in this case:

Screenshot 2025-01-24 at 13 26 07

So that its style aligns more with the paragraph styles?

Screenshot 2025-01-24 at 13 27 34

Copy link
Contributor

@stephanie-anderson stephanie-anderson left a 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!

jan-auer added a commit that referenced this pull request Jan 28, 2025
…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)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Feb 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants