Skip to content

Conversation

@brandon-pereira
Copy link
Member

@brandon-pereira brandon-pereira commented Dec 12, 2025

This PR removes bootstrap-icons entirely from the app. It also adds an eslint plugin to detect uses and throw an error, this will help in the immediate short term with PRs in flight and merging downstream.

Fixes HDX-3050

@brandon-pereira brandon-pereira requested review from a team and dhable and removed request for a team December 12, 2025 23:48
@changeset-bot
Copy link

changeset-bot bot commented Dec 12, 2025

🦋 Changeset detected

Latest commit: 3276ffc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Dec 12, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Dec 15, 2025 5:06pm

@github-actions
Copy link
Contributor

github-actions bot commented Dec 12, 2025

E2E Test Results

All tests passed • 46 passed • 3 skipped • 261s

Status Count
✅ Passed 46
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 3

View full report →

@claude
Copy link

claude bot commented Dec 12, 2025

Code Review

No critical issues found.

This PR successfully migrates from Bootstrap Icons to Tabler Icons. The changes are comprehensive and well-executed:

Strengths:

  • Complete removal of bootstrap-icons dependencies from HTML and SCSS
  • Consistent icon sizing (mostly 14-16px)
  • Added ESLint rule to prevent accidental use of bi- classes in future PRs
  • Proper accessibility: converted title attributes to aria-label where appropriate
  • Updated documentation (tech_stack.md) to reflect icon library change
  • Test files updated to match new icon implementations

Minor observations (non-blocking):

  • The ESLint rule uses a temporary comment noting it will be removed after PRs merge - this is a good safeguard
  • Icon size=14 is used consistently across most components, maintaining visual consistency

The migration is complete and ready to merge.

Copy link
Contributor

@pulpdrew pulpdrew left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this! It looks good except for a few minor size and alignment issues

@brandon-pereira
Copy link
Member Author

@pulpdrew all issues have been addressed except the webhook icon - I'll make a ticket for this!

@kodiakhq kodiakhq bot merged commit 4ba37e5 into main Dec 15, 2025
8 of 9 checks passed
@kodiakhq kodiakhq bot deleted the feature/eslint-warn-on-bootstrap-icons branch December 15, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants