-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Update default props #2926
Update default props #2926
Changes from all commits
3c1bda0
1c04fa7
1c334d4
60f3890
cbe5bbe
c31e17c
1546a9b
d006b8c
37b27c8
a6de7e6
3a8a3d0
faae8c5
94354c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
"react", | ||
"import" | ||
], | ||
"parser": "@babel/eslint-parser", | ||
"rules": { | ||
"accessor-pairs": ["error"], | ||
"block-scoped-var": ["error"], | ||
|
@@ -43,7 +44,7 @@ | |
"import/named": ["off"], | ||
"import/namespace": ["off"], | ||
"import/no-duplicates": ["error"], | ||
"import/no-named-as-default": ["error"], | ||
"import/no-named-as-default": ["off"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. apologies, but can you please explain what the impact of this one will be? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a linting that conflicted with the default notation on param functions. |
||
"import/no-unresolved": ["off"], | ||
"new-cap": ["error", { | ||
"capIsNewExceptionPattern": "Immutable\\.*" | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,8 +22,8 @@ const getSpinner = spinnerType => | |
const Loading = ({ | ||
children, | ||
loading_state, | ||
display, | ||
color, | ||
display = 'auto', | ||
color = '#119DFF', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we have colors and other constants scattered elsewhere through the code as well or is there a way to consolidate them all in one place? (very low priority, not worth holding up this PR for) |
||
id, | ||
className, | ||
style, | ||
|
@@ -32,10 +32,10 @@ const Loading = ({ | |
overlay_style, | ||
fullscreen, | ||
debug, | ||
show_initially, | ||
show_initially = true, | ||
type: spinnerType, | ||
delay_hide, | ||
delay_show, | ||
delay_hide = 0, | ||
delay_show = 0, | ||
target_components, | ||
custom_spinner, | ||
}) => { | ||
|
@@ -164,15 +164,6 @@ const Loading = ({ | |
|
||
Loading._dashprivate_isLoadingComponent = true; | ||
|
||
Loading.defaultProps = { | ||
type: 'default', | ||
color: '#119DFF', | ||
delay_show: 0, | ||
delay_hide: 0, | ||
show_initially: true, | ||
display: 'auto', | ||
}; | ||
|
||
Loading.propTypes = { | ||
/** | ||
* The ID of this component, used to identify dash components | ||
|
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.
this seems kind of important :-)
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.
I changed this to get tests to pass, but @T4rk1n can you confirm if this is OK?
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.
Yes that is the good type.