- 
                Notifications
    
You must be signed in to change notification settings  - Fork 111
 
feat(pci-kubernetes): delete shadcn, debug and install ods 19 #19444
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
feat(pci-kubernetes): delete shadcn, debug and install ods 19 #19444
Conversation
c3970d4    to
    dfd2ff3      
    Compare
  
    dfd2ff3    to
    e24a8d6      
    Compare
  
    | onClick, | ||
| }: TPciCardProps) => { | ||
| // TODO : fix badge background color with tailwind | ||
| // TODO : replace #f3fdff by --ods-color-information-025 when the custom property is fixed | 
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.
TODO ?
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.
line 29 : waiting for Tailwind update by CT 👉 normal
line 30 : should be removed by ODS update to v19.1.0 soon
| className={clsx( | ||
| ' flex flex-col items-center text-center px-[24px] py-[16px] min-h-[200px] rounded-md ', | ||
| )} | 
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.
suggestion : clsx seems useless here
| ODS_TEXT_SIZE, | ||
| } from '@ovhcloud/ods-components'; | ||
| import { OsdsMessage, OsdsSpinner, OsdsText } from '@ovhcloud/ods-components/react'; | ||
| import { OsdsButton, OsdsMessage, OsdsSpinner, OsdsText } from '@ovhcloud/ods-components/react'; | 
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.
suggestion: you should migrate thoses components to ODS19 (since it's the pr purpose)
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 purpose of the PR is to install ODS 19
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.
Ok
But you added OdsButton, why not add it in ODS19 instead ? Expecially since you already have Badge and Icon in ODS19
e24a8d6    to
    448ecd9      
    Compare
  
    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.
nice move ;) few adjustments to make and also, I think that your commit doesn't handle ODS19 yet, but just remove datatr-ux right? if it's the case, you should rename your commit and also remove the datatr-ux dependencies in package.json
58bedbc    to
    8d9c2b4      
    Compare
  
    8d9c2b4    to
    fb82a74      
    Compare
  
    9b44ab1    to
    06e28b1      
    Compare
  
    fb82a74    to
    25f6489      
    Compare
  
    06e28b1    to
    30898b7      
    Compare
  
    25f6489    to
    0c229e6      
    Compare
  
    ref: #TAPC-5164 Signed-off-by: Pierre-Philippe <[email protected]>
9273b82    to
    9f698f8      
    Compare
  
    9f698f8    to
    d6f7e35      
    Compare
  
    
ref: #TAPC-5001
Description
Ticket Reference: #...
Additional Information