mirror of
https://github.com/sourcegraph/sourcegraph.git
synced 2026-02-06 17:11:49 +00:00
Move "Testing web code" from handbook to doc/dev (#18777)
* Move "Testing web code" from handbook to doc/dev This is a follow-up to https://github.com/sourcegraph/about/pull/2569 * Add SVG files * Move 'Styling UI' guide over from handbook
This commit is contained in:
parent
7a768f6dea
commit
be35cda429
77
doc/dev/background-information/flat_call_tree.svg
Normal file
77
doc/dev/background-information/flat_call_tree.svg
Normal file
@ -0,0 +1,77 @@
|
||||
<svg width="440" height="108" viewBox="0 0 440 108" xmlns="http://www.w3.org/2000/svg">
|
||||
<defs>
|
||||
<marker id="arrow" markerWidth="10" markerHeight="10" refX="0" refY="3" orient="auto" markerUnits="strokeWidth">
|
||||
<path class="arrow" d="M0 0v6l9-3z" />
|
||||
</marker>
|
||||
</defs>
|
||||
<style>
|
||||
text {
|
||||
font-family: system-ui, sans-serif;
|
||||
}
|
||||
.control-flow-label {
|
||||
font-size: 8px;
|
||||
fill: darkblue;
|
||||
text-anchor: end;
|
||||
opacity: 0.6;
|
||||
}
|
||||
.control-flow-line {
|
||||
stroke: darkblue;
|
||||
stroke-opacity: 0.6;
|
||||
stroke-width: 0.5;
|
||||
stroke-dasharray: 1.5;
|
||||
fill: none;
|
||||
}
|
||||
.arrow {
|
||||
fill: darkblue;
|
||||
fill-opacity: 0.6;
|
||||
}
|
||||
.unit {
|
||||
fill: lightgrey;
|
||||
}
|
||||
.unit-label {
|
||||
font-size: 16px;
|
||||
text-anchor: middle;
|
||||
}
|
||||
.unit-connection {
|
||||
stroke: darkgrey;
|
||||
}
|
||||
</style>
|
||||
<line class="unit-connection" x1="220" x2="40" y1="40" y2="68" />
|
||||
<line class="unit-connection" x1="220" x2="112" y1="40" y2="68" />
|
||||
<line class="unit-connection" x1="220" x2="328" y1="40" y2="68" />
|
||||
<line class="unit-connection" x1="220" x2="184" y1="40" y2="68" />
|
||||
<line class="unit-connection" x1="220" x2="400" y1="40" y2="68" />
|
||||
<line class="unit-connection" x1="220" x2="256" y1="40" y2="68" />
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="188" y="8" />
|
||||
<text class="unit-label" x="220" y="30">A</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="8" y="68" />
|
||||
<text class="unit-label" x="40" y="90">B</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="80" y="68" />
|
||||
<text class="unit-label" x="112" y="90">C</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="152" y="68" />
|
||||
<text class="unit-label" x="184" y="90">D</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="224" y="68" />
|
||||
<text class="unit-label" x="256" y="90">E</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="296" y="68" />
|
||||
<text class="unit-label" x="328" y="90">F</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="368" y="68" />
|
||||
<text class="unit-label" x="400" y="90">G</text>
|
||||
</g>
|
||||
<g>
|
||||
<path class="control-flow-line" marker-end="url(#arrow)" d="M188 36c-10 7-36 8-56 11L49 60c-6 1-15 5-14 12 1 5 8 4 12 1 6-4 14-5 19-7 32-8 62-8 94-16 14-3 32-8 35-11 3-4 9-4 6 0-2 4-30 11-48 16s-46 8-47 15c0 1 1 4 4 3l28-9c13-5 29-7 42-11l21-9c7-4 7-10 12-7 2 4-12 12-19 19l-12 9c-4 3-6 5-1 8 6-1 29-21 32-26l5-8c0-1 2-2 3 0 2 3 2 6 5 9 8 8 20 15 28 24h4c2 0 3-3 2-5-4-7-17-14-26-21-5-4-7-5-7-8 0-2 3-2 5 0 3 3 2 3 12 10 9 6 76 19 82 23 4 2 8 2 8-2 0-5-60-16-90-24-3-1-5-2-8-6-1-3 3-3 4-1 2 4 4 3 9 5 18 6 52 12 72 16 20 5 59 9 73 13 6 2 9 1 11-3 0-5-4-5-9-6-15-4-52-8-61-9-38-5-68-10-80-17" />
|
||||
<text class="control-flow-label" x="150" y="41">control flow</text>
|
||||
</g>
|
||||
</svg>
|
||||
|
After Width: | Height: | Size: 2.8 KiB |
@ -15,6 +15,7 @@
|
||||
- [Developing the web clients](web/index.md)
|
||||
- [Developing the web app](web/web_app.md)
|
||||
- [Developing the code host integrations](web/code_host_integrations.md)
|
||||
- [Styling UI](web/styling.md)
|
||||
- [Developing the GraphQL API](graphql_api.md)
|
||||
- [Developing campaigns](campaigns/index.md)
|
||||
- [Developing code intelligence](codeintel/index.md)
|
||||
@ -45,6 +46,7 @@
|
||||
- [Continuous Integration](continuous_integration.md)
|
||||
- [How to run tests](testing.md)
|
||||
- [Testing Principles](testing_principles.md)
|
||||
- [Testing web code](testing_web_code.md)
|
||||
|
||||
## Tools
|
||||
|
||||
|
||||
80
doc/dev/background-information/nested_call_tree.svg
Normal file
80
doc/dev/background-information/nested_call_tree.svg
Normal file
@ -0,0 +1,80 @@
|
||||
<svg width="440" height="160" viewBox="0 0 440 160" xmlns="http://www.w3.org/2000/svg">
|
||||
<defs>
|
||||
<marker id="arrow" markerWidth="10" markerHeight="10" refX="0" refY="3" orient="auto" markerUnits="strokeWidth">
|
||||
<path class="arrow" d="M0 0v6l9-3z"></path>
|
||||
</marker>
|
||||
</defs>
|
||||
<style>
|
||||
text {
|
||||
font-family: system-ui, sans-serif;
|
||||
}
|
||||
.control-flow-label {
|
||||
font-size: 8px;
|
||||
fill: darkblue;
|
||||
text-anchor: end;
|
||||
opacity: 0.6;
|
||||
}
|
||||
.control-flow-line {
|
||||
stroke: darkblue;
|
||||
stroke-opacity: 0.6;
|
||||
stroke-width: 0.5;
|
||||
stroke-dasharray: 1.5;
|
||||
fill: none;
|
||||
}
|
||||
.arrow {
|
||||
fill: darkblue;
|
||||
fill-opacity: 0.6;
|
||||
}
|
||||
.unit {
|
||||
fill: lightgrey;
|
||||
}
|
||||
.unit-label {
|
||||
font-size: 16px;
|
||||
text-anchor: middle;
|
||||
}
|
||||
.unit-connection {
|
||||
stroke: darkgrey;
|
||||
}
|
||||
</style>
|
||||
|
||||
<line class="unit-connection" x1="220" x2="140" y1="40" y2="64"></line>
|
||||
<line class="unit-connection" x1="140" x2="100" y1="96" y2="120"></line>
|
||||
<line class="unit-connection" x1="140" x2="180" y1="96" y2="120"></line>
|
||||
<line class="unit-connection" x1="300" x2="260" y1="96" y2="120"></line>
|
||||
<line class="unit-connection" x1="300" x2="340" y1="96" y2="120"></line>
|
||||
<line class="unit-connection" x1="220" x2="300" y1="40" y2="64"></line>
|
||||
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="188" y="8"></rect>
|
||||
<text class="unit-label" x="220" y="30">A</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="108" y="64"></rect>
|
||||
<text class="unit-label" x="140" y="86">B</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="68" y="120"></rect>
|
||||
<text class="unit-label" x="100" y="142">C</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="148" y="120"></rect>
|
||||
<text class="unit-label" x="180" y="142">D</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="268" y="64"></rect>
|
||||
<text class="unit-label" x="300" y="86">E</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="228" y="120"></rect>
|
||||
<text class="unit-label" x="260" y="142">F</text>
|
||||
</g>
|
||||
<g>
|
||||
<rect class="unit" width="64" height="32" x="308" y="120"></rect>
|
||||
<text class="unit-label" x="340" y="142">G</text>
|
||||
</g>
|
||||
|
||||
<g>
|
||||
<path class="control-flow-line" marker-end="url(#arrow)" d="M 195 39 C 170 47 144 55 134 62 C 124 70 126 82 120 92 C 114 102 95 115 93 122 C 90 128 101 131 110 124 C 119 116 131 93 141 93 C 150 92 164 113 172 120 C 180 127 188 126 185 120 C 181 114 165 100 158 91 C 151 83 150 75 160 66 C 170 56 198 39 220 39 C 242 39 272 55 282 65 C 293 75 292 84 285 93 C 277 102 259 116 255 122 C 252 127 262 129 270 123 C 279 118 293 103 303 103 C 312 103 324 118 331 123 C 338 128 343 125 340 119 C 337 113 323 100 318 92 C 313 83 313 73 304 65 C 295 58 272 48 252 40"></path>
|
||||
<text class="control-flow-label" x="168" y="45">control flow</text>
|
||||
</g>
|
||||
</svg>
|
||||
|
After Width: | Height: | Size: 2.9 KiB |
@ -1,6 +1,6 @@
|
||||
# Testing
|
||||
|
||||
_This documentation is specifically for the tests in the [sourcegraph/sourcegraph](https://github.com/sourcegraph/sourcegraph) repository. For our general testing principles, please see "[Testing Principles](testing_principles.md)".
|
||||
_This documentation is specifically for the tests in the [sourcegraph/sourcegraph](https://github.com/sourcegraph/sourcegraph) repository. For our general testing principles, please see "[Testing Principles](testing_principles.md)"._
|
||||
|
||||
## Backend tests
|
||||
|
||||
|
||||
182
doc/dev/background-information/testing_web_code.md
Normal file
182
doc/dev/background-information/testing_web_code.md
Normal file
@ -0,0 +1,182 @@
|
||||
# Testing web code
|
||||
|
||||
This is the implementation of our [Testing Principles](testing_principles.md) for all code (TypeScript, stylesheets, ...) used on the web, such as our webapp, browser extension, shared code, or Sourcegraph extensions.
|
||||
|
||||
## General Goals
|
||||
|
||||
1. **Our product and code works as intended when we ship it to customers.**
|
||||
|
||||
2. **Our product and code doesn't accidentally break as we make changes over time.**
|
||||
|
||||
3. **Failures are clear about _what_ is not working and give as many hints as possible to _why_ it's not working.**
|
||||
|
||||
4. **Tests do not commonly need manual updates when the implementation is changed.**
|
||||
If tests commonly need manual updates, it creates the potential to accidentally change the test so that it doesn't catch a regression introduced in the same change. It also puts more burden on the code reviewers. In addition, updating tests can feel daunting and reduces the motivation to make changes to that part of the codebase.
|
||||
|
||||
5. **Tests are easy to write and maintain.**
|
||||
|
||||
## Testing UI Code
|
||||
|
||||
When testing UI, there are several categories of things that should be tested.
|
||||
These categories may be more or less important depending on the piece of UI.
|
||||
|
||||
### Visual regressions
|
||||
|
||||
A visual regression is a bug where the component behaves correctly, but no longer looks as intended.
|
||||
We use Percy screenshot tests to catch these.
|
||||
Percy can take screenshots in end-to-end tests, integration tests and in Storybooks.
|
||||
Storybook tests can be seen as a form of unit test for a component.
|
||||
Any story that is added to our codebase is automatically screenshotted on every CI build (but only in its initial state).
|
||||
|
||||
### Render output
|
||||
|
||||
Depending on the component, it can be important to test the _render output_ of the component.
|
||||
This encompasses the DOM structure and that the correct props are passed to other components.
|
||||
For example, when a component renders a link, we want to make sure right destination is passed.
|
||||
|
||||
This is accomplished by Jest snapshot tests, using a test renderer for React like `enzyme`.
|
||||
While this will inevitably also test parts that would be considered internal implementation details of the component, it generally has a better cost-benefit ratio than the alternative of plucking out specific elements and writing manual assertions.
|
||||
|
||||
Testing internal implementation details is generally bad because it is at odds with goal (4).
|
||||
Since snapshots are updated automatically though, this concern is not a problem in practice.
|
||||
Manual assertions on the other hand can cause that issue: Finding the right element often depends on targeting it with a CSS selector, which means the test can fail and require manual updates from changing the styling or DOM structure in unrelated ways.
|
||||
|
||||
In addition, it is often important not just to test what is rendered, but also _what is not rendered_.
|
||||
For example, after changing a condition in JSX, an additional element may be rendered that should not be there (or an undesired prop).
|
||||
This is only possible by taking the whole output into account, not by only making assertions on specific parts of it.
|
||||
|
||||
Snapshot diffs should be reviewed in pull request reviews the same way any other code or test diff should is reviewed and can be useful to highlight changes in render output to reviewers.
|
||||
|
||||
### Behavior
|
||||
|
||||
Interactive components can require tests of behavior.
|
||||
Enzyme allows [simulating events on elements](https://enzymejs.github.io/enzyme/docs/api/ReactWrapper/simulate.html), for example a click.
|
||||
[Sinon](https://sinonjs.org/) can be used to assert a callback prop is called in response to that, or Jest snapshots to assert the render output changes accordingly.
|
||||
|
||||
Behaviors involving many components can be tested well in integration tests with Puppeteer to make sure they are wired up correctly.
|
||||
Documentation for integration tests in our main repository can be found in the [repository's developer documentation](https://docs.sourcegraph.com/dev/background-information/testing#browser-based-tests).
|
||||
|
||||
## Writing testable code
|
||||
|
||||
### Avoid globals
|
||||
|
||||
Relying on global variables makes dependencies hard to stub for testing (a variable exported from a module counts as a global).
|
||||
It also means figuring out all transitive dependencies that need stubbing for a test is difficult.
|
||||
Even if it results in more code, prefer passing in dependencies as parameters (or props for React components), which makes dependencies explicit.
|
||||
|
||||
### Favor flat & wide call trees over deep & narrow call trees
|
||||
|
||||
When writing deeply nested call trees, a test for a unit in the middle of the tree inevitably also tests every layer beneath it.
|
||||
This means when a layer below is changed or breaks, the tests for the layers above it will likely break too.
|
||||
It also means that the test doesn't just need to pass stubs for the immediate dependencies of the unit, but also for the dependencies of every unit beneath it in the call tree.
|
||||
This directly means that if the interface of any unit beneath it changes, the tests of all units above it need to change too, violating goal (4).
|
||||
|
||||
<img alt="Nested call tree diagram" src="./nested_call_tree.svg" width="100%" />
|
||||
|
||||
In this simplified example call graph, to test unit B, we need to provide stubs for all the dependencies of C and D in addition to those of B itself.
|
||||
If the interface of those dependencies for C or D change, we now need to update all unit tests of B in addition to those of C or D.
|
||||
The tests for B and E also redundantly test C, D, F, and G.
|
||||
If something is broken in C, D, F or G, it will likely also cause B and E to be broken, leading to test failures that are red herrings on the path to find the root cause.
|
||||
|
||||
Instead, strive to write functions that take only the data they need and return data that can be "piped" to the next function.
|
||||
The top level takes care of composing these functions.
|
||||
This way each function can be tested individually with minimal stubbing effort and minimal chance of violating goals (3) and (4).
|
||||
|
||||
<img alt="Flat call tree diagram" src="./flat_call_tree.svg" width="100%" />
|
||||
|
||||
In the updated example call graph, to test any of the units B-G, we now only need to pass stubs for their own respective dependencies.
|
||||
If any of their interfaces change, it will only affect the unit tests of direct dependents.
|
||||
|
||||
For React components, this can mean writing components that take **children** instead of hard-coding them, which also makes snapshot tests assert fewer implementation details.
|
||||
|
||||
For RxJS code, this can mean implementing logic as _operators_ (functions taking an Observable and returning an Observable).
|
||||
|
||||
## Guidelines for writing tests
|
||||
|
||||
### Use specific assertion functions
|
||||
|
||||
Using specific assertion functions gives better error messages and insights when a test is failing into _why_ is it failing.
|
||||
|
||||
Examples:
|
||||
|
||||
- Use `assert.equal(value, 123)`/`expect(value).toBe(123)` instead of `assert(value === 123)`/`expect(value === 123).toBeTrue()`.
|
||||
The former will throw an assertion error like "expected 456 to be 123", while the latter will throw an unhelpful error like "expected false to be true".
|
||||
|
||||
- Use `assert.deepStrictEqual(object, { property: 123 })`/`expect(object).toEqual({ property: 123 })` over `assert.equal(object.property, 123)`/`expect(object.property).toBe(123)`.
|
||||
The former will print a diff of the actual and expected objects, while the latter may only throw an error like "expected undefined to equal 123".
|
||||
|
||||
- Use [`sinon.assert.calledOnce(spy)`](https://sinonjs.org/releases/latest/assertions/) instead of `expect(calledTimes).toBe(1)`.
|
||||
`expect(calledTimes).toBe(1)` will throw an assertion error "expected 0 to be 1", while `sinon` will give something like "expected function subscribe to be called 1 time, but was called 0 times".
|
||||
|
||||
- Use [`expectObservable()`](https://rxjs-dev.firebaseapp.com/guide/testing/marble-testing) instead of `sinon.assert.calledOnce(subscribe)`.
|
||||
If the `subscribe` callback is not called at all for example because it errored, `expectObservable()` will print a diff of the expected and actual emissions, including that error, which can give hints at the cause of the failure. `sinon` here would just fail with "expected function to be called once, but was called 0 times", which does not give any hint _why_ it failed.
|
||||
|
||||
- In rare cases where none of these are possible and boolean `assert()` is the only option, pass a custom assertion message.
|
||||
|
||||
### Avoid putting too much logic into test files
|
||||
|
||||
The more logic is in the test itself, the more likely it is there is a bug in the test itself.
|
||||
If the logic is unavoidable, it may be moved into an external function outside the test file.
|
||||
This also intentionally makes that utility subject to have tests itself (it will be included in coverage tracking since the file name will not end in `.test.ts`).
|
||||
|
||||
### Keep tests focused
|
||||
|
||||
It may be tempting to make many assertions as possible in one test to save repeating setup code.
|
||||
Instead of making many assertions that test different things in one test, favor splitting the test into multiple.
|
||||
For example, favor three separate tests for "Add X", "Update X" and "Remove X" with fewer assertions each over a single test like "CRUD feature X works" with many assertions.
|
||||
This ensures that when one of them fails, the developer debugging can see whether the other two features are still working and therefor better narrow down where the problem is.
|
||||
Use [`beforeEach()`/`afterEach()` hooks](https://jestjs.io/docs/en/setup-teardown) to avoid duplication of common setup and teardown logic and giving better errors when setup or teardown fails (as opposed to the test itself).
|
||||
|
||||
### Write clear test titles
|
||||
|
||||
Give tests titles that describe the behavior being tested of the unit like a specification (not unclear titles like "it works").
|
||||
When tests fail, it should be clear which exact behaviors of the unit under test are broken (and which are not).
|
||||
|
||||
Prefer using the `it()` test function over `test()` as it affords wording the title like a specification of behavior (it affords following "it" with a verb, while `test()` does not).
|
||||
When using `it()`, the title should read like an English statement in present tense starting with "it", where "it" refers to the unit under test.
|
||||
|
||||
Avoid repetition in test titles by nesting tests within `describe()` blocks.
|
||||
For example, instead of a test suite like this:
|
||||
|
||||
```ts
|
||||
describe('ExampleDetailsComponent', () => {
|
||||
it('shows the details of the example', ...)
|
||||
it('shows an edit button if in view mode and user has permissions', ...)
|
||||
it('shows a delete button if in view mode and user has permissions', ...)
|
||||
it('submits the form if submit button is clicked and in edit mode', ...)
|
||||
it('discards the form if discard button is clicked and in edit mode', ...)
|
||||
// ...
|
||||
})
|
||||
```
|
||||
|
||||
group tests like this:
|
||||
|
||||
```ts
|
||||
describe('ExampleDetailsComponent', () => {
|
||||
describe('view mode', () => {
|
||||
it('shows the details of the example', ...)
|
||||
describe('user has edit permissions', () => {
|
||||
it('shows an edit button if user has permissions', ...)
|
||||
it('shows a delete button if user has permissions', ...)
|
||||
})
|
||||
})
|
||||
describe('edit mode', () => {
|
||||
it('submits the form if submit button is clicked', ...)
|
||||
it('discards the form if discard button is clicked', ...)
|
||||
})
|
||||
// ...
|
||||
})
|
||||
```
|
||||
|
||||
This also creates an opportunity to e.g. define common props for this context to avoid duplication.
|
||||
|
||||
### Treat tests with the same standards to type safety as application code
|
||||
|
||||
Using a static type system like TypeScript helps in detecting errors early and makes refactoring easier.
|
||||
This is equally true for test code as it is for implementation code.
|
||||
When defining stub objects to pass to a unit under test it may be tempting to only define the exact methods the unit is currently using and casting the object (to `any` or the required type) to circumvent the resulting compile errors.
|
||||
This however quickly violates goals (3) and (4), as it will swallow type errors when making incompatible changes to the implementation in the future.
|
||||
It also means automatic refactors like a simple "Rename symbol" will no longer rename the symbol everywhere (making the test fail and requiring a manual update).
|
||||
|
||||
Instead, strive to provide compliant stub implementations (this also excludes throwing a "Not implemented" error, but noop methods can be okay if they satisfy the type).
|
||||
If this causes you to implement a lot of methods that are not actually used, consider refactoring the unit under test to not require those dependencies.
|
||||
112
doc/dev/background-information/web/styling.md
Normal file
112
doc/dev/background-information/web/styling.md
Normal file
@ -0,0 +1,112 @@
|
||||
# Styling UI
|
||||
|
||||
Sourcegraph has many UI components. A unique constraint for these is that they need to run within different _environments_.
|
||||
Wherever a component is running, it should look native to its environment and consistent with the design of that environment.
|
||||
For example, our hover overlay needs to work and _behave_ the same in the Sourcegraph webapp and in the browser extension, where it is injected into a variety of code hosts, but _look_ native to the environment.
|
||||
Components need to be able to adapt styles from CSS stylesheets already loaded on the page (no matter how those were architected).
|
||||
|
||||
## Goals
|
||||
|
||||
1. Components decoupled from styling, that look **consistent** with the host environment.
|
||||
2. Support light and dark themes.
|
||||
3. Tooling support:
|
||||
- Autocompletion for styles when writing components
|
||||
- Autocompletion when writing styles
|
||||
- Linting for styles
|
||||
- Browser dev tools to easily inspect and iterate on styles
|
||||
- Autoprefixer
|
||||
4. Support for advanced CSS features, like state selectors, pseudo elements, flexbox, grid, media queries, CSS variables, ...
|
||||
|
||||
## Environments
|
||||
|
||||
### Host-agnostic UI
|
||||
|
||||
Components that need to run in different environments (any UI shared between our browser extension and the webapp) adopt styles from their environments through configurable CSS class names (as opposed to trying to replicate the styling with copied CSS).
|
||||
A component may accept multiple class names for different elements of the component.
|
||||
An example of this is `<HoverOverlay/>`: see how [the different props it accepts](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@4047521a92904054e782d341001d08d61945c86f/-/blob/shared/src/hover/HoverOverlay.tsx#L27-39) for its child components' class names are passed in the [webapp](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@4047521a92904054e782d341001d08d61945c86f/-/blob/web/src/components/shared.tsx#L35-44) and in [code host integrations](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@4047521a92904054e782d341001d08d61945c86f/-/blob/browser/src/shared/code-hosts/shared/codeHost.tsx#L443:1).
|
||||
|
||||
### Host-specific UI
|
||||
|
||||
In the environments we control ourselves (such as our webapp, the options page of the browser extension, or our marketing website), we use a customized version of [Bootstrap](https://getbootstrap.com/) as a CSS framework.
|
||||
Any code inside our webapp can and should make use of the CSS classes that Bootstrap provides as building blocks (and should generally do so instead of writing custom styles).
|
||||
This includes classes like [cards](https://getbootstrap.com/docs/4.5/components/card/), [buttons](https://getbootstrap.com/docs/4.5/components/buttons/) or [input groups](https://getbootstrap.com/docs/4.5/components/input-group/), but also utility classes for [layout](https://getbootstrap.com/docs/4.5/utilities/flex/) and [spacing](https://getbootstrap.com/docs/4.5/utilities/spacing/).
|
||||
Please refer to the excellent [Bootstrap documentation](https://getbootstrap.com/docs/4.5/) for everything that is available for use.
|
||||
To see what our customizations look like visually (in both light and dark theme), you can find a [showcase in our Storybook](https://5f0f381c0e50750022dc6bf7-oozlspcdwk.chromatic.com/?path=/story/web-global-styles).
|
||||
|
||||
Components only used in a specific host environment do not need to support customization through class names.
|
||||
They can however utilize environment-agnostic components by passing our Bootstrap classes as custom `className` values.
|
||||
|
||||
## Our approach to styling
|
||||
|
||||
### Structuring style sheets
|
||||
|
||||
A component may need styles that are common to all environments, like internal layout.
|
||||
We write those styles in SCSS stylesheets that are imported into the host environment.
|
||||
In some cases these can be overridden by passing another class name for that element.
|
||||
|
||||
To avoid naming conflicts we structure these files using the [BEM convention](http://getbem.com/naming/) (Block - Element - Modifier).
|
||||
The _block_ name is always the React component name, _elements_ and _modifiers_ are used as specified in BEM.
|
||||
A _block_ must not be referenced in any other React component than the one with the matching name.
|
||||
|
||||
Example:
|
||||
|
||||
```scss
|
||||
.some-component {
|
||||
// .. styles ...
|
||||
|
||||
&__element {
|
||||
// ... styles ...
|
||||
|
||||
&--modifier {
|
||||
// ... styles ...
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- **Block**: A React component name in kebab-case. This class is always assigned to the root DOM element of the component.
|
||||
- **Element**: A sub-element of the component. This should be a name that describes the semantic of this element within the component.
|
||||
- **Modifier**: A modifier of the _element_, e.g. `--loading` or `--closed`. This is only rarely needed.
|
||||
|
||||
Please note that there is no hierarchy in _elements_, as that would couple the styling to the DOM structure. Element names should be unambiguous within their component/_block_, or be split into a separate component/_block_.
|
||||
|
||||
We colocate stylesheets next to the component using the same file name.
|
||||
This approach ensures styles are easy to find and makes it easy to tell which styles apply to which elements (by putting them side-by-side in an editor, or through a simple text search).
|
||||
|
||||
Using classes, as opposed to child and descendant selectors, decouples styles from the DOM structure of the component, ensures encapsulation and avoids CSS specificity issues.
|
||||
Descendant selectors can still be useful in rare cases though, like styling in the browser extension, or styling markdown content.
|
||||
|
||||
### Typography
|
||||
|
||||
Avoid ever overriding font family, text sizes or text colors.
|
||||
These are set globally by the host environment for semantic HTML elements, e.g. `<h1>`, `<a>`, `<code>` or `<small>`.
|
||||
|
||||
### Colors and theming
|
||||
|
||||
The brand color palette is [OpenColor](https://yeun.github.io/open-color/).
|
||||
In addition to these, we define a blueish grayscale palette for backgrounds, text and borders.
|
||||
These colors are all available as SCSS and CSS variables.
|
||||
However, directly referencing these may not work well in both light and dark themes, and may not match code host themes (if the component is shared).
|
||||
The best approach is to not reference colors at all and use building blocks that have borders, text colors etc defined.
|
||||
This saves code and makes it easy to maintain design consistency even if we want to change colors in the future.
|
||||
When that is not possible (for example UI contributed by extensions), prefer to reference CSS variables with semantic colors like `var(--danger)`, `var(--success)`, `var(--border-color)`, `var(--body-bg)` etc.
|
||||
The values of these variables are changed globally when the theme changes.
|
||||
Be aware that this means our stylesheets for each host environment need to define these variables too.
|
||||
|
||||
Defining different styles in the webapp depending on the `theme-dark` and `theme-light` classes predates CSS variables and is discouraged.
|
||||
|
||||
### Spacing
|
||||
|
||||
We use `rem` units in all component styling and strive to use `0.25rem` steps.
|
||||
This ensures our spacing generally aligns with an [8pt grid](https://medium.com/swlh/the-comprehensive-8pt-grid-guide-aa16ff402179), but also gracefully scales in environments that have a different base `rem` size.
|
||||
In our webapp, it is recommended to make use of [Bootstrap's margin and padding utilities](https://getbootstrap.com/docs/4.5/utilities/spacing/), which are configured to align with the 8pt grid.
|
||||
|
||||
### Layout
|
||||
|
||||
We use modern CSS for our layouting needs. You can find a [small playground in our Storybook](https://5f0f381c0e50750022dc6bf7-oozlspcdwk.chromatic.com/?path=/story/web-global-styles--layout). The dev tools of modern browsers provide a lot of useful tooling to work with CSS layouts.
|
||||
|
||||
Layouts should always be _responsive_ to make sure Sourcegraph is usable with different screen resolutions and window sizes, e.g. when resizing the browser window and using Sourcegraph side-by-side with an editor.
|
||||
|
||||
[CSS Flexbox](https://css-tricks.com/snippets/css/a-guide-to-flexbox/) is used for **one-dimensional** layouts (single rows or columns, with optional wrapping). In the webapp, you can use utility classes for simple flexbox layouts and responsive layouts. This is the most common layout method.
|
||||
|
||||
For complex **two-dimensional** layouts, [CSS Grid](https://learncssgrid.com/) can be used.
|
||||
Loading…
Reference in New Issue
Block a user