Skip to content

feat(next-js): New NextJS example page - #899

Merged
ArthurKnaus merged 5 commits into
masterfrom
aknaus/feat/next-js/new-example-page
Mar 25, 2025
Merged

feat(next-js): New NextJS example page#899
ArthurKnaus merged 5 commits into
masterfrom
aknaus/feat/next-js/new-example-page

Conversation

@ArthurKnaus

@ArthurKnaus ArthurKnaus commented Mar 24, 2025

Copy link
Copy Markdown
Member

Redesign the example page for NextJS.

Screenshot 2025-03-24 at 12 58 33

Deployed a preview here:
https://next-js-sentry-xi.vercel.app/sentry-example-page

Note:
Once getsentry/sentry-javascript#15780 is done, I will revisit this and show the adblocker warning only when needed.

@ArthurKnaus
ArthurKnaus requested review from a team and lforst March 24, 2025 12:02
@github-actions

github-actions Bot commented Mar 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against b2ff5f7

@codecov

codecov Bot commented Mar 24, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.83%. Comparing base (7edb3f4) to head (b2ff5f7).
Report is 4 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff            @@
##           master     #899    +/-   ##
========================================
  Coverage   51.83%   51.83%            
========================================
  Files          53       53            
  Lines        3654     3654            
  Branches      864      852    -12     
========================================
  Hits         1894     1894            
- Misses       1631     1759   +128     
+ Partials      129        1   -128
Flag Coverage Δ
e2e-tests 80.28% <ø> (ø)
unit-tests 50.68% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ArthurKnaus ArthurKnaus changed the title feat(next-js): New example page feat(next-js): New NextJS example page Mar 24, 2025
lforst
lforst approved these changes Mar 24, 2025
View reviewed changes

@lforst lforst left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👌

Comment thread src/nextjs/templates.ts
Comment on lines +265 to 267
<p className="description">
Adblockers will prevent errors from being sent to Sentry.
</p>

@lforst lforst Mar 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pragmatic - I like

@lforst

lforst commented Mar 24, 2025

Copy link
Copy Markdown

The black-on-green text for the notification is maybe a bit stark. Do we have a color theme here we could apply?

@lforst

lforst commented Mar 24, 2025

Copy link
Copy Markdown

Honestly, now that I see this page, I am not sure whether we really need something like isProbablyUsingAdBlock(). The warning is really not intrusive and visible enough for people to see.

@ArthurKnaus

ArthurKnaus commented Mar 24, 2025

Copy link
Copy Markdown
Member Author

Honestly, now that I see this page, I am not sure whether we really need something like isProbablyUsingAdBlock(). The warning is really not intrusive and visible enough for people to see.

@lforst we think it would be better UX to show an error message and omit the success message in case the SDK is blocked somehow. Showing success + not working is simply confusing (even with disclaimer).

@ArthurKnaus
ArthurKnaus merged commit af92743 into master Mar 25, 2025
@ArthurKnaus
ArthurKnaus deleted the aknaus/feat/next-js/new-example-page branch March 25, 2025 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@lforst lforst lforst approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ArthurKnaus @lforst