Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the sidebar flag parameters in the Map class to provide more flexible configuration of sidebar behavior. The changes allow both add_sidebar and add_floating_sidebar parameters to default to None, with logic to determine their values based on user input or use sensible defaults.
Key Changes:
- Changed default values for
add_sidebarandadd_floating_sidebarfrom explicit booleans toNone - Added conditional logic to handle parameter combinations and set appropriate values
- Maintains backward compatibility by defaulting to the original behavior when both parameters are
None
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| add_sidebar: bool = None, | ||
| add_floating_sidebar: bool = None, |
There was a problem hiding this comment.
Type annotations should be Optional[bool] instead of bool since the default value is now None. This ensures type checkers correctly recognize that these parameters can accept None.
| elif add_sidebar: | ||
| add_floating_sidebar = False | ||
| elif add_floating_sidebar: | ||
| add_sidebar = False |
There was a problem hiding this comment.
The conditional logic doesn't handle the case where both add_sidebar and add_floating_sidebar are explicitly set to True by the user. When both are True, both elif branches execute sequentially, resulting in both being set to False, which is likely unintended. Consider adding validation to prevent conflicting parameters or handle this case explicitly.
|
🚀 Deployed on https://69189b09872fbc13f0a26aa1--opengeos.netlify.app |
* Update sidebar flag * Fix type hints
No description provided.