chore: improve content of frontend contributing guide #14619
Conversation
| - **components** - Reusable UI components without Coder specific business | ||
| logic |
There was a problem hiding this comment.
@jaaydenh @aslilac @BrunoQuaresma This was actually something I was going to ask about for this week's frontend variety, but do you think that going forward, we should enforce a "no data fetching in the components directory" rule?
We currently have some for the filter options and the autocomplete components, but my hope would be either:
- Move these to modules
- Decouple the layout concerns for the components from the fetching, and move the fetching somewhere else
There was a problem hiding this comment.
The filter options logic technically isn't hitting an API endpoint itself, but it's still doing weird stuff like putting JSX directly inside the query cache
There was a problem hiding this comment.
The filter component is a bad example 😓, so we shouldn't consider it a best practice. However, I do think it makes sense not to have fetching logic inside 'shareable' components.
There was a problem hiding this comment.
@Parkreiner Do you think there is anything specific to call out in the docs about this right now?
docs/contributing/frontend.md
Outdated
| should always try to use the base components that are provided by the library or | ||
| from the codebase. It's recommended that you always do a quick search before | ||
| creating a custom primitive component like dialogs, popovers, buttons, etc. | ||
| Do not assume existing components and modules have been categorized correctly. |
There was a problem hiding this comment.
| Do not assume existing components and modules have been categorized correctly. | |
| Our codebase has some legacy components that are being updated to follow these new conventions, but all new components should follow these guidelines. |
There was a problem hiding this comment.
Feel like the current wording is throwing the team under the bus a bit
There was a problem hiding this comment.
@Parkreiner hehe, you are totally correct, I was just trying to be really clear about the current state of things to avoid confusion. But I agree your wording sounds much better.
There was a problem hiding this comment.
I wanted to create another PR eventually to move some components to modules.
| In both cases, you can access the dashboard on `http://localhost:8080`. If you | ||
| are running the `./scripts/develop.sh` you can log in using the default | ||
| credentials: `admin@coder.com` and `SomeSecurePassword!`. | ||
| > **Default Credentials:** `admin@coder.com` and `SomeSecurePassword!`. |
There was a problem hiding this comment.
nit: this is not quite the GHFM "callout" that @Parkreiner was suggesting, this is just a block quote, which is also kind of a misrepresentation
| making API requests. The API functions are centralized in `site/src/api/api.ts`. | ||
| Auto-generated TypeScript types derived from our Go server are located in | ||
| `site/src/api/typesGenerated.ts`. |
There was a problem hiding this comment.
although it looks like we're inconsistent about it... sigh
The goal of this update are:
This refactor does not address the testing section
In the future, more information about theming should be included in the guide.