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

adjusting handleDynamicStyle #332

Merged
merged 2 commits into from
Oct 9, 2024
Merged

Conversation

BSd3v
Copy link
Collaborator

@BSd3v BSd3v commented Oct 7, 2024

adjusting handleDynamicStyle to check if the params.data exists before running the test

closes #321

@BSd3v BSd3v requested review from ndrezn and AnnMarieW October 7, 2024 18:02
@ndrezn ndrezn requested a review from T4rk1n October 7, 2024 19:51
Copy link
Collaborator

@AnnMarieW AnnMarieW left a comment

Choose a reason for hiding this comment

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

Looks like a good solution 🙂

Copy link
Member

@ndrezn ndrezn left a comment

Choose a reason for hiding this comment

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

💃 with a small recommendation

Comment on lines +1123 to 1129
if (params) {
if (params.data) {
if (test(params)) {
return style;
}
}
}
Copy link
Member

@ndrezn ndrezn Oct 9, 2024

Choose a reason for hiding this comment

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

It's syntax sugar, but you can remove a nested if with:

Suggested change
if (params) {
if (params.data) {
if (test(params)) {
return style;
}
}
}
if (params?.data)
if (test(params)) {
return style;
}
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This doesn't lint, otherwise I would have done it.

Copy link
Member

Choose a reason for hiding this comment

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

Plausibly because we're supporting old versions of JS. Feel free to ignore this comment then and merge!

@BSd3v BSd3v merged commit f240ce6 into plotly:main Oct 9, 2024
3 checks passed
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.

Dash AG Grid bug in getRowStyle with infinite scroll
3 participants