Skip to content

Props deprecations - CardImage, CardSection, StateScreen #1023

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

Merged
merged 5 commits into from
Nov 16, 2020

Conversation

Inbal-Tish
Copy link
Collaborator

Description

Starting deprecation process for CardImage, CardSection, StateScreen with deprecation warning and support for old and new props.

Changelog

CardImage, CardSection, StateScreen - Adding props deprecation warnings.

@Inbal-Tish Inbal-Tish requested a review from ethanshar November 8, 2020 10:31
@Inbal-Tish Inbal-Tish added the Important for Next Release PR that must be included in the release version label Nov 8, 2020
@ethanshar ethanshar requested review from M-i-k-e-l and removed request for ethanshar November 13, 2020 08:05
@ethanshar ethanshar assigned ethanshar and M-i-k-e-l and unassigned ethanshar Nov 13, 2020
…precated props from demo screens; adding 'source' to StateScreen typings
@Inbal-Tish Inbal-Tish requested a review from M-i-k-e-l November 15, 2020 13:38
LogService.warn(
'uilib: Please stop passing borderRadius to Card.Image, it will get the borderRadius from the Card'
);
LogService.deprecationWarn({component: 'CardImage', oldProp: 'borderRadius'});
Copy link
Collaborator

@M-i-k-e-l M-i-k-e-l Nov 15, 2020

Choose a reason for hiding this comment

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

That's a good idea to add this option, but the current message expects the newProp (actually this is my bad, I need to fix LogService typings).
You can use the warn method for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My suggestion is that we change the LogService deprecationWarn's massage to accommodate this scenario. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added this option in the other PR

@M-i-k-e-l
Copy link
Collaborator

Looks like you need to run npm run build:dev

@Inbal-Tish
Copy link
Collaborator Author

Inbal-Tish commented Nov 16, 2020

Done

@M-i-k-e-l M-i-k-e-l merged commit fa0447c into master Nov 16, 2020
@Inbal-Tish Inbal-Tish deleted the infra/add_deprecation_warnings branch November 16, 2020 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Important for Next Release PR that must be included in the release version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants