-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
dcc: refactor slider #3414
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
dcc: refactor slider #3414
Conversation
5626ff1 to
0fab988
Compare
|
@T4rk1n please review (and complete if necessary) - we want this for Studio. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put the test files in tests/unit, I just find it easier after that to manage test runners in ci/pre-commit if they are separated from the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Moved to tests/unit.
| persistence_type = PersistenceTypes.local, | ||
| // eslint-disable-next-line no-magic-numbers | ||
| verticalHeight = 400, | ||
| ...rest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest -> props dedupe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deduped.
| enum PersistenceTypes { | ||
| 'local' = 'local', | ||
| 'session' = 'session', | ||
| 'memory' = 'memory', | ||
| } | ||
|
|
||
| enum PersistedProps { | ||
| 'value' = 'value', | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those will be the same for all the components that use ts, we can put them in the type.ts file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
757d7cf to
7909533
Compare
7cbd4f4 to
9e9ccf9
Compare
b8a0d71 to
076fa7a
Compare
T4rk1n
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💃
In this PR:
Contributor Checklist
optionals
CHANGELOG.md