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: replace widget class by antd Card component #159

Closed

Conversation

shootermv
Copy link
Contributor

@shootermv shootermv commented Oct 29, 2023

Description

i can see widget css class, functioning as - what can be called a card effect
i think it can be better to use antd library Card component

screenshots

image

@shootermv shootermv requested a review from NoamGaash as a code owner October 29, 2023 05:36
@netlify
Copy link

netlify bot commented Oct 29, 2023

Deploy Preview for astonishing-pothos-5f81bd ready!

Name Link
🔨 Latest commit f31f60d
🔍 Latest deploy log https://app.netlify.com/sites/astonishing-pothos-5f81bd/deploys/6542251ac8d74d0008ee11ab
😎 Deploy Preview https://deploy-preview-159--astonishing-pothos-5f81bd.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@shootermv shootermv changed the title refactor: replace widget by antd Card component refactor: replace widget class by antd Card component Oct 29, 2023
@NoamGaash
Copy link
Member

why is it better?

@shootermv
Copy link
Contributor Author

shootermv commented Oct 29, 2023 via email

@NoamGaash
Copy link
Member

NoamGaash commented Oct 29, 2023

I appreciate the idea, but I don't think we really need this component.
The way I see it,

  1. The more we use antd and mui components, the harder it will be to change or remove things in the future
  2. Those components encapsulates behaviours that we're not necessarily aware of, and I'm afraid of changes

Good refactor serves some higher level goal, such as

  • Simplify the code
  • Improve readability
  • Improve code consistency (for example = naming conventions)
  • remove code duplication
    and so on

I'm open to another opinions, I don't want to be too fixed on mine

image

@shootermv shootermv force-pushed the replace-widget-elements-by-ant-card-component branch from f3fd36a to cfda9d7 Compare November 1, 2023 10:14
@shootermv shootermv force-pushed the replace-widget-elements-by-ant-card-component branch from cfda9d7 to f31f60d Compare November 1, 2023 10:14
@shootermv shootermv closed this Nov 7, 2023
@shootermv shootermv deleted the replace-widget-elements-by-ant-card-component branch February 7, 2024 13:03
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.

2 participants