chore: OPTIC-1313: Reducing Toast usage in django templates to get rid of toast.min.js #6604
Conversation
…d of toast.min.js
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for heartex-docs canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #6604 +/- ##
========================================
Coverage 76.63% 76.63%
========================================
Files 169 169
Lines 13840 13840
========================================
Hits 10606 10606
Misses 3234 3234
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
So cool! |
label_studio/templates/base.html
Outdated
There was a problem hiding this comment.
typo in the name of param is app settings: should be persistEnce
| @@ -0,0 +1 @@ | |||
| export { ReactComponent as LsCross } from "./cross.svg"; | |||
There was a problem hiding this comment.
| export { ReactComponent as LsCross } from "./cross.svg"; | |
| export { ReactComponent as IconCross } from "./cross.svg"; |
There was a problem hiding this comment.
Since this is now getting used from the proper place, the name needs to reflect properly what it is. Ls is a namespace based on the application not a particular to the function. Can we update this accordingly here and also let's maybe document this somewhere in the Guild initiatives to call this out.
There was a problem hiding this comment.
I also considered having LSE using this version of cross.svg - but i noticed that the 2 versions of cross.svg are actually different - which is mildly concerning - I'll have one of the designers chime in on that - would be nice to double down on standardization
| @@ -1,24 +1,30 @@ | |||
| import { Toast, ToastAction, type ToastProps } from "../../components/Toast/Toast"; | |||
| import { Toast, ToastAction, ToastType, type ToastProps } from "../Toast/Toast"; | |||
| import { LsCross } from "../../assets/icons"; | |||
| import type { FC } from "react"; | |||
| import { Block } from "../../utils/bem"; | |||
There was a problem hiding this comment.
BEM cannot be used in the ui library.
There was a problem hiding this comment.
I had reservations on this tbh, but this ticket has already exploded in scope so i opted to go down this route. you are right though i'll get rid of BEM in Toast and MessageToast in favor of scss modules
web/libs/ui/src/utils/bem.tsx
Outdated
There was a problem hiding this comment.
This should not be added to the UI library.
PR fulfills these requirements
[fix|feat|ci|chore|doc]: TICKET-ID: Short description of change madeex.fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errorsChange has impacts in these area(s)
(check all that apply)
Describe the reason for change
reducing relyance on toast.min.js to reduce the need for the extra file