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.

Sign up for GitHub

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

Jump to bottom

RND-168 Dell Icons - part 2 - export ts object #164

Closed
wants to merge 6 commits into from
Closed

RND-168 Dell Icons - part 2 - export ts object #164

wants to merge 6 commits into from

Conversation

leon-good-life
Copy link
Contributor

@leon-good-life leon-good-life commented Apr 27, 2023 •
edited
Loading

Description

follow up to #157

PR in stage: cloudify-cosmo/cloudify-stage#2494

Screenshots / Videos

Screenshot 2023-04-27 at 13 21 07

Checklist

Tests

N/A

Documentation

Will be done at the same time of finalising the PR in cloudify-tage.

@leon-good-life leon-good-life added the feature Pull request with non-breaking change adding new or enhancing already existing functionality label Apr 27, 2023
@leon-good-life leon-good-life requested a review from qooban April 27, 2023 10:41
@leon-good-life leon-good-life mentioned this pull request Apr 27, 2023
Closed
4 tasks
@leon-good-life leon-good-life requested review from Vorbert-Kruk and qooban and removed request for qooban and Vorbert-Kruk April 27, 2023 12:12
Vorbert-Kruk
Vorbert-Kruk reviewed Apr 27, 2023
View reviewed changes
packages/frontend/styles/ddsIcons.ts Outdated Show resolved Hide resolved
packages/frontend/styles/ddsIcons.ts Outdated Show resolved Hide resolved
packages/frontend/styles/ddsIcons.ts Outdated Show resolved Hide resolved
packages/frontend/styles/ddsIcons.ts Outdated Show resolved Hide resolved
@qooban qooban removed their request for review April 28, 2023 13:15
@qooban qooban assigned kubama and unassigned kubama Apr 28, 2023
@qooban qooban requested a review from kubama April 28, 2023 13:15
@qooban
Copy link
Contributor

qooban commented Apr 28, 2023 •
edited by jira bot
Loading

Sorry, I have to opt out as I'm leaving for 1-week vacation. Please work with @kubama .

I guess this PR can help in RND-563.

kubama
kubama reviewed May 4, 2023
View reviewed changes
Copy link
Contributor

@kubama kubama left a comment

Choose a reason for hiding this comment

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

I don't think this PR is the way to go, and as such I'd rather reject it, at least for now.
Please have a look at https://github.com/cloudify-cosmo/cloudify-ui-components/tree/RND-463 - we are going to wrap the whole dds-core library, which contains already all we need (see https://github.com/cloudify-cosmo/cloudify-ui-components/blob/a0468e445ac00dcb7ccea53c22089d95f276c0e9/dds-core/src/components/icon/ts/icon.ts). I don't see a reason to duplicate things.
To keep things simple, I think all icon related stuff should be exposed as part of yet to come dds Icon react component.
As a side node - looks like the same area is now being covered by different squads, and there is no communication or agreement how to approach things. To bad.

@leon-good-life leon-good-life deleted the RND-168-part-2 branch May 4, 2023 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kubama kubama kubama left review comments

@Vorbert-Kruk Vorbert-Kruk Vorbert-Kruk left review comments

Assignees
No one assigned
Labels
feature Pull request with non-breaking change adding new or enhancing already existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@leon-good-life @qooban @Vorbert-Kruk @kubama