Skip to content

Wildcard: Migrate usage of "badge" styles to `<Badge />` component

Warren Gifford requested to merge tr/wildcard-badge-migration into main

Created by: umpox

Badge migration

This PR is a complete migration from the Bootstrap badge className usage to the Wildcard <Badge /> component.

Migration walkthrough

This PR is also intended to serve as a test of a typical Wildcard migration that can be used for reference when starting future migrations. Here's a quick run through of what needs to be done:

  1. Add a valid eslint rule to error on any usage of the old approach. For example: https://github.com/sourcegraph/sourcegraph/blob/5728c53fdb930f3773954c75678b77f62297dd59/client/eslint-plugin-wildcard/lib/index.js#L29-L38
  2. This will help us find anything that should be immediately migrated to the new approach. Look at the eslint output from the above change here: https://buildkite.com/sourcegraph/sourcegraph/builds/118401#fb4a76d9-6e18-4e02-9e0b-28cea946006d/146-151
  3. That isn't guaranteed to capture everything to be migrated. We need to ensure we migrate any cases where we pass the className through as a separate prop (e.g. badgeClassName). Use a Sourcegraph search to find existing usage, like this: https://sourcegraph.com/search?q=context:global+repo:%5Egithub%5C.com/sourcegraph/sourcegraph%24%408e5fcfb+lang:TypeScript+%22badge%22+OR+%22badge-%22&patternType=regexp&case=yes
  4. We shouldn't have a need for global Bootstrap classes anymore. Our aim now should be to remove this dependency if possible. Create a CSS module with classes that we need from node_modules/bootstrap/scss/someComponent.scss and variables from client/branded/src/global-styles/someComponent.scss. If the Bootstrap SCSS file uses a lot of mixins that are difficult to copy over, you can use the generated node_modules/bootstrap/dist/css/bootstrap.css file for some raw CSS.

Notes

  • We might find we need to modify the original component to support an existing style or approach. That's OK, we should aim to keep these changes fairly simple though. If a component needs a larger refactor, it might be better to raise a separate PR.
  • We should not aim to address any UI changes in these migrations. We should ensure that these PRs preserve as much of the original UI as possible. The exception for this will be when we actually want to change the UI, this should be clear in the relevant issue for this work.
  • In some cases the component might be shared across our browser extension code and our web code. The browser extension code can not use the branded styles, we must update the component to allow exposing an unstyled variant that can be configured with props. If this becomes problematic, raise this with the rest of the frontend platform team. We have a possible solution for this through this issue: https://github.com/sourcegraph/sourcegraph/issues/19140

Checklist

  • Migrate Bootstrap "badge" usage to <Badge />
  • Fix minor UI issues
  • Enable eslint rule to ban Bootstrap "badge" global className
  • Move global badge styles to CSS module
  • Remove Bootstrap badge import

closes https://github.com/sourcegraph/sourcegraph/issues/27622

Merge request reports