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

**Breaking:** Remove deprecated constants #929

Merged
merged 17 commits into from
Mar 12, 2019
Merged

Conversation

TejasQ
Copy link
Contributor

@TejasQ TejasQ commented Feb 21, 2019

⚠️ THIS PR IS VERY LIKELY TO HAVE VISUAL REGRESSIONS ⚠️

PLEASE @micha-f and @contiamo/frontend, spend a few minutes and go through each component to see which sizes/paddings/etc. broke.

The components whose snapshots changed need extra attention.

It may be helpful to have (side by side) the PR preview and master. I am doing the same.

This PR removes all of the unnecessary dead weight in the library.

Related issue

To be tested

Me

  • No error or warning in the console on localhost:6060

Tester 1

  • Things look good on the demo.

Tester 2

  • Things look good on the demo.

@TejasQ TejasQ self-assigned this Feb 21, 2019
@TejasQ TejasQ marked this pull request as ready for review February 21, 2019 10:03
@fabien0102
Copy link
Contributor

image

This example is broken, even on master, let's remove the old API 😅

@fabien0102
Copy link
Contributor

image

This should be green

@fabien0102
Copy link
Contributor

image

The toggle switch is broken

Copy link
Contributor

@fabien0102 fabien0102 left a comment

Choose a reason for hiding this comment

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

Some components to fix

@micha-f
Copy link
Member

micha-f commented Feb 26, 2019

I think we need a thorough review of all components before we can merge this. @fabien0102 did you go through all components? Or is your feedback just the result of a quick check?

@TejasQ TejasQ force-pushed the remove-deprecated-constants branch from 95628ce to 4b2656b Compare March 11, 2019 14:39
@TejasQ TejasQ force-pushed the remove-deprecated-constants branch from 817b699 to c1b18c0 Compare March 11, 2019 16:11
@TejasQ TejasQ force-pushed the remove-deprecated-constants branch from c1b18c0 to e23fdbf Compare March 11, 2019 16:45
Copy link
Contributor

@stereobooster stereobooster left a comment

Choose a reason for hiding this comment

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

I haven't found any visual regressions

@TejasQ TejasQ merged commit 08b7f61 into master Mar 12, 2019
@TejasQ TejasQ deleted the remove-deprecated-constants branch March 12, 2019 10:49
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.

4 participants