-
-
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
changed typecheck for list value to use isinstance instead of type #3108
base: dev
Are you sure you want to change the base?
Conversation
@T4rk1n this one looks good to me, but can you please check it before we merge? |
I'd rather see the check fixed than change the layout. The check at Line 2257 in bbd013c
use a brittle/unoptimal API to check for ids that is outdated (doesn't work with component as props). dash/dash/development/base_component.py Line 366 in bbd013c
The issue here is that we end up with a list of list so the traverse can't find it here: Lines 2250 to 2255 in bbd013c
Need to extract the layout value then concat directly it if it's a list if not wrap in a list. |
…s id content check
.gitignore
Outdated
app.py | ||
launch.json |
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'd recommend just avoiding committing these files rather than changing .gitignore
; while unlikely we may need these file names in the future.
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.
Hi @ndrezn will remove them from gitignore and make sure not to commit the files. Thank you.
Hi @T4rk1n, I made the changes you recommended, please check. I'd just like to ask if all the integration tests need to remain unchanged or would I need to change them based on my changes? Because I see even on dev branch the integration tests are failing (unless I'm wrong). Is this correct, or can I go ahead and modify these failing integration tests? |
Solves: Layout as list of components does not work with Dash Pages page_container #2924
I've checked if the values passed to layout setter isinstance list, if so, set the values to html.Div (dash Component)
Contributor Checklist
optionals
CHANGELOG.md