-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(adr): add proposals for client architecture and slash roadmap #783
Open
JLou
wants to merge
1
commit into
main
Choose a base branch
from
adr-14-02
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+150
−0
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
72 changes: 72 additions & 0 deletions
72
docs/decision-records/0003-client-packages-architecture.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
# Architecture of the client packages to support multiple themes | ||
|
||
## Status | ||
|
||
Accepted on 14/02/2025. Present at the meeting: Samuel Gomez, Johnathan Meunier, | ||
Martin Stievenart, Guillaume Kesterman, Paul Plancq, Jean-Lou Piermé. | ||
|
||
## Context | ||
|
||
Our design system needs to support 2 themes for the client oriented | ||
applications: Look and feel, and Apollo. The proposal made by Samuel Gomez is to | ||
create a new package for each theme. The architecture looks like this | ||
|
||
``` | ||
client | ||
├───look-and-feel | ||
│ ├───css | ||
│ │ └───src | ||
│ │ └───Button | ||
│ │ Button.scss | ||
│ └───react | ||
│ └───src | ||
│ └───Button | ||
│ Button.tsx | ||
├───apollo | ||
│ ├───css | ||
│ │ └───src | ||
│ │ └───Button | ||
│ │ Button.scss | ||
│ └───react | ||
│ └───src | ||
│ └───Button | ||
│ Button.tsx | ||
└───common | ||
└───src | ||
└───Button | ||
Button.scss | ||
Button.tsx | ||
``` | ||
|
||
In the common folder, code that can be shared between the two themes will be | ||
stored. For the necessary adjustments, each theme can override code. Ideally, | ||
the in the theme source files, we would only re-export the common component. | ||
|
||
Four package would be created: | ||
|
||
- @design-system/client-look-and-feel-css | ||
- @design-system/client-look-and-feel-react | ||
- @design-system/client-apollo-css | ||
- @design-system/client-apollo-react | ||
|
||
### Pros | ||
|
||
- Most of the code would be common, only variants not found in a specific theme | ||
would be developped inside their own theme, and minor visual changes would be | ||
done in the respective css package. | ||
|
||
### Cons | ||
|
||
- More packages to maintain pipelines for | ||
- Build is a tad more complex | ||
|
||
## Decision | ||
|
||
A unanimous decision was made to follow the proposed architecture. | ||
|
||
## Consequences | ||
|
||
- The architecture will be updated to reflect the new structure. | ||
- The new packages will be created. | ||
- The new packages will be published to the npm registry. | ||
- Build pipelines will be updated to include the new packages. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
# Technical roadmap for slash in 2025 | ||
|
||
## Status | ||
|
||
Accepted on 14/02/2025. Present at the meeting: Samuel Gomez, Johnathan Meunier, | ||
Martin Stievenart, Guillaume Kesterman, Paul Plancq, Jean-Lou Piermé. | ||
|
||
## Context | ||
|
||
Our slash codebase has been around since 2019 and much has changed in the | ||
frontend, and react worlds. Some of the technique we had to use back then are | ||
obsolete now, and we have to adapt to the new standards. | ||
|
||
We also have to think about the future, and how we can make our codebase more | ||
maintainable, while avoiding large breaking changes each version. | ||
|
||
### 1.1.0 - Immediate future | ||
|
||
Our #1 priority is to support react 19. As soon as we are confident with it, a | ||
1.1.0 will be shipped. | ||
|
||
### 2.0.0 - Variants | ||
|
||
The code has been using BEM and `classmodifier`s to handle variants. This has | ||
been working well, but there are some challenges. The main one being that typing | ||
`classmodifier` is impossible. Using variants, we can leverage TypeScript's | ||
union types to type our variants. It would align with what is done in the more | ||
recent client codebase. | ||
|
||
Example: | ||
|
||
```tsx | ||
// In 1.x | ||
<Button classModifier="secondary">Secondary button</Button> | ||
|
||
// In 2.x | ||
<Button variant="secondary">Secondary button</Button> | ||
``` | ||
|
||
Removing `classModifier` means we need to add a way for our users to inject | ||
custom classes into our components. This can be done by using the `className` | ||
which will not remove base classes anymore. | ||
|
||
```tsx | ||
|
||
// In 1.x | ||
<Button className="my-custom-class">button</Button> // would render <button class="my-custom-class">Secondary button</button> | ||
|
||
// In 2.x | ||
<Button className="my-custom-class">button</Button> // would render <button class="button my-custom-class">Secondary button</button> | ||
``` | ||
|
||
This means a massive breaking change on all of our components, either with the | ||
removal of `classModifier` or the change in the `className` behavior. | ||
|
||
### 3.0.0 - Dropping bootstrap grid system | ||
|
||
We have been using the bootstrap grid system for a long time. It has been | ||
working well, but it is not the most efficient way to handle layout in 2025. | ||
With flexboxes, and grid there are more elegant ways to align our components. | ||
|
||
There is a massive unmaintained css file copied from bootstrap v4 that we can | ||
remove. | ||
|
||
Projects relying on `col-md-x` and others will have 2 options: | ||
|
||
- include that css file themselves | ||
- update their code to avoid relying on these classes | ||
|
||
## Decision | ||
|
||
A unanimous decision was made to make theses changes, in that order. | ||
|
||
## Consequences | ||
|
||
- Milestones will be created for each of the versions | ||
- A migration guide will be written for each of the versions | ||
- A post will be written to explain the changes to our users |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the in the theme source files
je sais pas trop ce que tu as voulu dire là