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(badge, emoji-tabset, empty-state): remove DtIcon #481

Merged

Conversation

juliodialpad
Copy link
Collaborator

Remove DtIcon usage on Badge, Emoji Tabset and Empty State components

Obligatory GIF (super important!)

Obligatory GIF

🛠️ Type Of Change

These types will increment the version number on release:

  • Refactor

📖 Jira Ticket

https://dialpad.atlassian.net/browse/DLT-1916

📖 Description

  • Removed DtIcon usage from Badge, Emoji tabset and Empty state components, changed to slots if any icon could be used or to specific icon if just a few needed for the component.

💡 Context

We're making dialtone-vue fully tree-shakeable

📝 Checklist

For all PRs:

  • I have ensured no private Dialpad links or info are in the code or pull request description (Dialtone is a public repo!).
  • I have reviewed my changes.
  • I have considered the performance impact of my change.

For all Vue changes:

  • I have added / updated unit tests.
  • I have made my changes in Vue 2 and Vue 3. Note: you may sync your changes from Vue 2 to Vue 3 (or vice versa) using the ./scripts/dialtone-vue-sync.sh script. Read docs here: Dialtone Vue Sync Script

🔮 Next Steps

  • Merge to beta and test

@juliodialpad juliodialpad added the no-visual-test Add this tag when the PR does not need visual testing label Aug 30, 2024
@juliodialpad juliodialpad self-assigned this Aug 30, 2024
@ninamarina
Copy link
Contributor

Some tests are failing. Seems like importing the icons this way it doesn't add the data-qa attribute that it used to.

@ninamarina
Copy link
Contributor

The controls in storybook are still showing a select menu even though now the icon and illustration are slots:
image

Same with the badge

@ninamarina
Copy link
Contributor

The controls in storybook are still showing a select menu even though now the icon and illustration are slots: image

Same with the badge

Never mind, I see that's on purpose just for the playground

@ninamarina
Copy link
Contributor

We have a new property "iconSize" for the badge component. Do we really want that? What would happen if a different size is passed? I don't see any changes in storybook
image

Copy link
Contributor

@ninamarina ninamarina left a comment

Choose a reason for hiding this comment

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

Looks good, I only have the question about the iconSize prop.

@braddialpad
Copy link
Contributor

braddialpad commented Sep 4, 2024

We have a new property "iconSize" for the badge component. Do we really want that? What would happen if a different size is passed? I don't see any changes in storybook

Wouldn't they just pass the size of the icon via the component they pass into the slot? Don't think the prop is necessary. Should we instead have a scoped slot for default size?

@juliodialpad
Copy link
Collaborator Author

You're right, I was thinking on making "easier" to handle both icon sizes with a default size of '200' but makes no sense to have a prop, will remove it.

Copy link
Contributor

@braddialpad braddialpad left a comment

Choose a reason for hiding this comment

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

The changes look pretty good to me, however we will need to update the docsite examples to reflect the new changes.

Copy link
Contributor

@braddialpad braddialpad left a comment

Choose a reason for hiding this comment

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

Looks good, you can merge this. Regarding the doc I mentioned you can do it in another PR if you wish.

@juliodialpad
Copy link
Collaborator Author

Looks good, you can merge this. Regarding the doc I mentioned you can do it in another PR if you wish.

Updated docs, don't want to forgot to do it later haha

Copy link
Contributor

@braddialpad braddialpad left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link

github-actions bot commented Sep 5, 2024

✔️ Deploy previews ready!
😎 Dialtone preview: https://dialtone.dialpad.com/deploy-previews/pr-481/
😎 Dialtone-vue 2 preview: https://dialtone.dialpad.com/vue/deploy-previews/pr-481/
😎 Dialtone-vue 3 the preview: https://dialtone.dialpad.com/vue3/deploy-previews/pr-481/

@juliodialpad juliodialpad merged commit 0fa1280 into remove/dt-icon Sep 5, 2024
11 checks passed
@juliodialpad juliodialpad deleted the remove/dt-icon--badge-emoji_tabset-empty_state branch September 5, 2024 18:28
juliodialpad pushed a commit that referenced this pull request Sep 5, 2024
# [9.73.0-beta.1](dialtone/v9.72.1...dialtone/v9.73.0-beta.1) (2024-09-05)

### Bug Fixes

* NO-JIRA merge staging into beta ([#478](#478)) ([09f1165](09f1165))
* **Rich Text Editor:** DLT-2017 emojis positioning ([#486](#486)) ([b857386](b857386))

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Avatar:** DLT-1916 remove dt-icon ([#466](#466)) ([6ef90db](6ef90db))
* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
juliodialpad pushed a commit that referenced this pull request Sep 5, 2024
# [2.159.0-beta.1](dialtone-vue2/v2.158.0...dialtone-vue2/v2.159.0-beta.1) (2024-09-05)

### Bug Fixes

* NO-JIRA merge staging into beta ([#478](#478)) ([09f1165](09f1165))
* **Rich Text Editor:** DLT-2017 emojis positioning ([#486](#486)) ([b857386](b857386))

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
juliodialpad pushed a commit that referenced this pull request Sep 5, 2024
# [3.152.0-beta.1](dialtone-vue3/v3.151.0...dialtone-vue3/v3.152.0-beta.1) (2024-09-05)

### Bug Fixes

* NO-JIRA merge staging into beta ([#478](#478)) ([09f1165](09f1165))
* **Rich Text Editor:** DLT-2017 emojis positioning ([#486](#486)) ([b857386](b857386))

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Avatar:** DLT-1916 remove dt-icon ([#466](#466)) ([6ef90db](6ef90db))
* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
juliodialpad pushed a commit that referenced this pull request Sep 23, 2024
# [9.77.0-beta.1](dialtone/v9.76.0...dialtone/v9.77.0-beta.1) (2024-09-23)

### Bug Fixes

* NO-JIRA merge staging into beta ([#478](#478)) ([09f1165](09f1165))
* **Tokens:** DLT-2053 android tokens color value ([#504](#504)) ([368c9db](368c9db))

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Avatar:** DLT-1916 remove dt-icon ([#466](#466)) ([6ef90db](6ef90db))
* **Avatar:** DLT-1916 remove dt-icon from Avatar vue 2 ([#474](#474)) ([e8600c3](e8600c3))
* **Feed Item Pill:** DLT-1916 remove dt-icon from Feed Item Pill ([#489](#489)) ([bde73c8](bde73c8))
* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
* **Message Input:** DLT-1916 remove dt-icon from message input ([#490](#490)) ([3215116](3215116))
juliodialpad pushed a commit that referenced this pull request Sep 23, 2024
# [2.162.0-beta.1](dialtone-vue2/v2.161.0...dialtone-vue2/v2.162.0-beta.1) (2024-09-23)

### Bug Fixes

* NO-JIRA merge staging into beta ([#478](#478)) ([09f1165](09f1165))

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Avatar:** DLT-1916 remove dt-icon from Avatar vue 2 ([#474](#474)) ([e8600c3](e8600c3))
* **Feed Item Pill:** DLT-1916 remove dt-icon from Feed Item Pill ([#489](#489)) ([bde73c8](bde73c8))
* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
* **Message Input:** DLT-1916 remove dt-icon from message input ([#490](#490)) ([3215116](3215116))
juliodialpad pushed a commit that referenced this pull request Sep 23, 2024
# [3.155.0-beta.1](dialtone-vue3/v3.154.0...dialtone-vue3/v3.155.0-beta.1) (2024-09-23)

### Bug Fixes

* NO-JIRA merge staging into beta ([#478](#478)) ([09f1165](09f1165))

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Avatar:** DLT-1916 remove dt-icon ([#466](#466)) ([6ef90db](6ef90db))
* **Avatar:** DLT-1916 remove dt-icon from Avatar vue 2 ([#474](#474)) ([e8600c3](e8600c3))
* **Feed Item Pill:** DLT-1916 remove dt-icon from Feed Item Pill ([#489](#489)) ([bde73c8](bde73c8))
* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
* **Message Input:** DLT-1916 remove dt-icon from message input ([#490](#490)) ([3215116](3215116))
juliodialpad pushed a commit that referenced this pull request Oct 1, 2024
# [9.77.0](dialtone/v9.76.3...dialtone/v9.77.0) (2024-10-01)

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Avatar:** DLT-1916 remove dt-icon ([#466](#466)) ([6ef90db](6ef90db))
* **Avatar:** DLT-1916 remove dt-icon from Avatar vue 2 ([#474](#474)) ([e8600c3](e8600c3))
* **Feed Item Pill:** DLT-1916 remove dt-icon from Feed Item Pill ([#489](#489)) ([bde73c8](bde73c8))
* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
* **Message Input:** DLT-1916 remove dt-icon from message input ([#490](#490)) ([3215116](3215116))
juliodialpad pushed a commit that referenced this pull request Oct 1, 2024
# [2.162.0](dialtone-vue2/v2.161.2...dialtone-vue2/v2.162.0) (2024-10-01)

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Avatar:** DLT-1916 remove dt-icon from Avatar vue 2 ([#474](#474)) ([e8600c3](e8600c3))
* **Feed Item Pill:** DLT-1916 remove dt-icon from Feed Item Pill ([#489](#489)) ([bde73c8](bde73c8))
* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
* **Message Input:** DLT-1916 remove dt-icon from message input ([#490](#490)) ([3215116](3215116))
juliodialpad pushed a commit that referenced this pull request Oct 1, 2024
# [3.155.0](dialtone-vue3/v3.154.2...dialtone-vue3/v3.155.0) (2024-10-01)

### Code Refactoring

* **Badge, Emoji Tabset, Empty State:** remove DtIcon ([#481](#481)) ([0fa1280](0fa1280))

### Features

* **Avatar:** DLT-1916 remove dt-icon ([#466](#466)) ([6ef90db](6ef90db))
* **Avatar:** DLT-1916 remove dt-icon from Avatar vue 2 ([#474](#474)) ([e8600c3](e8600c3))
* **Feed Item Pill:** DLT-1916 remove dt-icon from Feed Item Pill ([#489](#489)) ([bde73c8](bde73c8))
* **Keyboard Shortcut To Unread Pill:** DLT-1916 remove dt-icon ([#482](#482)) ([277ae13](277ae13))
* **Message Input:** DLT-1916 remove dt-icon from message input ([#490](#490)) ([3215116](3215116))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-visual-test Add this tag when the PR does not need visual testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants