-
Notifications
You must be signed in to change notification settings - Fork 102
Andarist/webext groundwork refactor #367
base: dev
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
); | ||
|
||
const dragMachine = dragModel.createMachine( | ||
{ |
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.
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.
I think this restructuring makes sense. And from what I can test at the preview deploy, it also looks to be working. And the only failing test is fixed upstream. So LGTM 🚢
Goals of this PR:
<WebExtension/>
that eg. doesn't depend on Monocao anyhow or doesn't know anything about embed mode etc<WebExtension/>
and also to migrate closer to a single source of truth for styling (previously we were mixing SCSS with Chakra)In general, to review this PR it might be the easiest to just follow its commits. Most are self-contained refactoring pieces, with some minor bug fixes at the end.
I'm not super fond of some of those "customizable" points here - perhaps this could be structured in a better way. I feel though that the current shape of the code is manageable and we could always refactor stuff later. If you have any general thoughts on composition patterns for problems like this - I'm happy to discuss different tradeoffs etc.