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

Bids 3014/edit webhook dialog #593

Merged
merged 11 commits into from
Aug 5, 2024
Merged

Conversation

marcel-bitfly
Copy link
Contributor

@marcel-bitfly marcel-bitfly commented Jul 18, 2024

  • update of nuxt
  • update of depending packages
  • refactorings due to errors caused by updates
  • some BaseComponents (e.g. Forms)
  • mechanism to make sure only the topmost Dialog of nested Dialogs (Dialogs opening Dialogs) gets closed
  • implementing edit webhook dialog
  • add validation object and schema factory (to abstract yup library)

@marcel-bitfly
Copy link
Contributor Author

marcel-bitfly commented Jul 18, 2024

We have to come up with new formatting rules, as I had to remove the deprecated nuxt 2 eslint formating rules 🙃

We could

(Also take into consideration: https://www.joshuakgoldberg.com/blog/configuring-eslint-prettier-and-typescript-together/#stop-using-eslint-for-formatting)

(nice to have, as we do not use a css utility framework : https://stylelint.io/)

@thib-wien
Copy link
Contributor

👋🏻
My overall comments on your PR:

  • you did a useful and courageous work, thanks for updating,
  • maybe separate the update from your dialog boxes,
  • you created small components for a button, a form, an input, but they are quite empty. If they are not meant to grow, writing their code directly in the parent file would decrease the already huge amount of files that we have,
  • I do not like these silencers for es-lint, it looks like bad practice (it is not your fault). Maybe we could relax some rules if Alexander finds some of them annoying, and open a BIDS to refactor the code so that we do not need the remaining silencers.

@marcel-bitfly marcel-bitfly force-pushed the BIDS-3014/edit-webhook-dialog branch 6 times, most recently from 82712cc to 64f90c6 Compare July 23, 2024 13:30
Copy link

cloudflare-workers-and-pages bot commented Jul 23, 2024

Deploying beaconchain with  Cloudflare Pages  Cloudflare Pages

Latest commit: 22f52ba
Status: ✅  Deploy successful!
Preview URL: https://0c4cfa6b.beaconchain.pages.dev
Branch Preview URL: https://bids-3014-edit-webhook-dialo.beaconchain.pages.dev

View logs

@marcel-bitfly marcel-bitfly force-pushed the BIDS-3014/edit-webhook-dialog branch 8 times, most recently from 4743f2d to 5a3ec02 Compare July 25, 2024 12:26
Copy link
Contributor

@MauserBitfly MauserBitfly left a comment

Choose a reason for hiding this comment

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

Quit some work you did there with all the eslint / types refactoring.

I like that you created some more base components.

The UI of the dialog already looks nice and works fine.

Left some comments for consideration / discussion ;-)

@marcel-bitfly marcel-bitfly force-pushed the BIDS-3014/edit-webhook-dialog branch 5 times, most recently from a2b6c60 to d53913f Compare July 30, 2024 09:25
@marcel-bitfly marcel-bitfly force-pushed the BIDS-3014/edit-webhook-dialog branch 5 times, most recently from d5423df to 4edd96d Compare August 1, 2024 10:59
@marcel-bitfly marcel-bitfly force-pushed the BIDS-3014/edit-webhook-dialog branch from 4edd96d to 86847ad Compare August 1, 2024 11:30
Copy link
Contributor

@MauserBitfly MauserBitfly left a comment

Choose a reason for hiding this comment

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

Thanks for the updates.

Still might consider changing this

And please remove the new console.log ;-)

@marcel-bitfly marcel-bitfly force-pushed the BIDS-3014/edit-webhook-dialog branch 3 times, most recently from b6d1765 to fcb82ec Compare August 2, 2024 10:21
frontend/components/bc/BcLink.vue Outdated Show resolved Hide resolved
frontend/components/bc/BcTranslation.vue Outdated Show resolved Hide resolved
frontend/components/bc/input/BcInputCheckbox.vue Outdated Show resolved Hide resolved
frontend/components/bc/BcTranslation.vue Show resolved Hide resolved
MarcelBitfly added 9 commits August 5, 2024 11:15
Due to the introduction of `nested css` sass had to change the be compliant.
So there should not be `declerations` after `nested code` or the code
should be wraped in '& {}' explicitly

 See: https://sass-lang.com/documentation/breaking-changes/mixed-decls/
Due to `typescript package updates` some imports had to by adapted.
Refactoring was necessary due to `nuxt/prefer-import-meta` eslint rule,
due to an upate
Refactoring was necessary due to `nuxt/prefer-import-meta` eslint rule,
due to an upate
Refactoring was necessary due to an upate
- due to conflicts the  `newest and recommended eslint integration` was added
- Locking `prime vue version` to `0.2.2` due to a `breaking change in styles`.
Might broke due to a `merge conflict`
Added undefined to `error description`
@marcel-bitfly marcel-bitfly force-pushed the BIDS-3014/edit-webhook-dialog branch from 9123f97 to 17ed892 Compare August 5, 2024 12:44
Copy link
Contributor

@MauserBitfly MauserBitfly left a comment

Choose a reason for hiding this comment

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

LGTM

MarcelBitfly added 2 commits August 5, 2024 14:56
Add `BaseComponents` like `Forms` and `Buttons`
Add `autocomplete` for `translations` via `useTranslations()`
Add `schema factory` and `validation object`to wrap `third party validation`

See: BIDS-3014
Arrow functions have been called in `watch` before `initialization`
@marcel-bitfly marcel-bitfly force-pushed the BIDS-3014/edit-webhook-dialog branch from 17ed892 to 22f52ba Compare August 5, 2024 12:58
@marcel-bitfly marcel-bitfly merged commit e0d9c21 into staging Aug 5, 2024
2 checks passed
@marcel-bitfly marcel-bitfly deleted the BIDS-3014/edit-webhook-dialog branch August 5, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants