Skip to content
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

refactor(ui): move to /ui folder, switch to vite for build #1581

Merged
merged 46 commits into from
Nov 10, 2022
Merged

Conversation

davinov
Copy link
Member

@davinov davinov commented Nov 4, 2022

Part of #912 was to move everything UI-related into its own folder, so the root stays cleans.
I took this opportunity to also switch the build system to vite (it became necessary because typescript needed to be updated too, and the rollup plugins were not following).

Sorry for this faaaaaaat PR, tat contained a lot of sed/replacements in order to clean all the build errors.

TODO:

  • finish cleaning error of types build
  • repair storybook
  • switch tests to vitest
  • deduplicate playground code
  • adapt github actions and sonar

@render
Copy link

render bot commented Nov 4, 2022

Note: the naming-convention may need some configuration
The store was not shared otherwise!
Use the alpha v7 to have manager and preview built by vite
I will not make them all! It's way too much work, we should do them piece by piece, only keeping ones that are relevant
These components are exposed in the lib,
so it's important to have means to see them work
in isolation.
Also fix a few typing issues
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Base: 98.41% // Head: 0.00% // Decreases project coverage by -98.41% ⚠️

Coverage data is based on head (3a5804d) compared to base (a0083cc).
Patch has no changes to coverable lines.

❗ Current head 3a5804d differs from pull request most recent head a6703e6. Consider uploading reports for the commit a6703e6 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #1581       +/-   ##
==========================================
- Coverage   98.41%       0   -98.42%     
==========================================
  Files         218       0      -218     
  Lines        6171       0     -6171     
  Branches      963       0      -963     
==========================================
- Hits         6073       0     -6073     
+ Misses         98       0       -98     
Impacted Files Coverage Δ
...erbird/src/components/stepforms/schemas/convert.ts
...rd/src/components/stepforms/schemas/concatenate.ts
...bird/src/components/stepforms/schemas/evolution.ts
...d/src/components/stepforms/schemas/aggregations.ts
weaverbird/src/components/stepforms/StepForm.vue
...c/components/stepforms/widgets/TotalDimensions.vue
...d/src/components/stepforms/DateExtractStepForm.vue
...verbird/src/components/DeleteConfirmationModal.vue
weaverbird/src/lib/dereference-pipeline.ts
...ird/src/components/stepforms/schemas/percentage.ts
... and 208 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@davinov
Copy link
Member Author

davinov commented Nov 9, 2022

OK, I think it's good enough for now. I prefer merging this ASAP to unblock the ability to do releases.
Need to keep in mind that:

  • A lot of typechecking errors are still there, there were not previously visible because the CI doesn't check for them. the migration to vue 2.7 also improved the typechecking.
  • A few tests were skipped (popover, filter editor). Their components works OK, I just hadn't had the time to figure their problem (which doesn't seem trivial).
  • Only some stories were migrated, there is still many to do!

Copy link
Contributor

@jGundermann jGundermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the great PR! Huge work congrats.
Got some type error during build. and found a minor change to make.

.gitignore Show resolved Hide resolved
@@ -60,7 +60,7 @@ import VariableInput from './VariableInput.vue';
FAIcon,
},
})
export default class AutocompleteWidget extends Mixins(FormWidget) {
export default class AutocompleteWidget extends FormWidget {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah oui t'as carrément enlever l'usage de Mixins! Bonne idée ^^

Copy link
Contributor

@bloodstorms bloodstorms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tout OK pour moi, testé en local super fast 🚀, tout fonctionne comme avant, pas d'erreur en console 👌
Let's meeeeeeerge

@davinov davinov merged commit 378158d into master Nov 10, 2022
@davinov davinov deleted the f/ui-org branch November 10, 2022 14:17
@davinov davinov mentioned this pull request Nov 10, 2022
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants