✨ FEATURE: Add editable argument to scenario_selector (#2562) #2786
Conversation
- Add optional editable property (default: True) - Hide edit icons when editable=False - Preserve all other functionality (selection, sorting, adding) - Add comprehensive tests and documentation - Maintain backward compatibility
|
Hi @arcanaxion , this is the implementation for #2562. Let me know if any changes are needed. |
FredLL-Avaiga
left a comment
FredLL-Avaiga
left a comment
There was a problem hiding this comment.
Well done @SushanthKS06
A few changes are needed
And some file organization ...
There was a problem hiding this comment.
please follow the naming convention used for the other components (ie <Component>.spec.tsx)
| updateScVars?: string; | ||
| showSearch?: boolean; | ||
| creationNotAllowed?: string; | ||
| editable?: boolean; |
There was a problem hiding this comment.
if property type is dynamic_boolean, we need 2 properties (defaultEditable & editable)
and read them with useDynamicProperty
check the implementation of the render property of the Alert component
There was a problem hiding this comment.
not sure where this file should go but I guess not here
@FabienLelaquais ?
There was a problem hiding this comment.
not sure where this file should go but I guess not here
@FabienLelaquais ?
There was a problem hiding this comment.
not sure where this file should go but I guess not here
@FabienLelaquais ?
There was a problem hiding this comment.
not sure where this file should go but I guess not here
@FabienLelaquais ?
There was a problem hiding this comment.
not sure where this file should go but I guess not here
@FabienLelaquais ?
FredLL-Avaiga
added
Core
There was a problem hiding this comment.
Pull request overview
This PR adds an optional editable boolean parameter to the scenario_selector visual element, allowing developers to control whether users can edit scenario names and tags. When set to False, the edit UI controls are hidden while preserving other functionality like selection, sorting, and scenario creation.
Key changes:
- Added
editableproperty to backend configuration with default value ofTruefor backward compatibility - Modified frontend to conditionally render edit component based on the property value
- Included comprehensive test coverage for both backend and frontend
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| taipy/gui_core/viselements.json | Added editable property definition with type dynamic(bool) and default True |
| taipy/gui_core/_GuiCoreLib.py | Registered editable as PropertyType.dynamic_boolean with default True |
| frontend/taipy/src/ScenarioSelector.tsx | Added editable prop and conditional rendering of edit component |
| frontend/taipy/src/ScenarioSelector.editable.test.tsx | Frontend tests for editable functionality (contains bugs) |
| tests/gui_core/test_scenario_selector_editable.py | Python unit tests for the editable property |
| tests/gui_core/test_scenario_selector_editable_integration.py | Python integration tests |
| validate_json_only.py | Validation script (should be removed from production) |
| validate_editable_implementation.py | Validation script (should be removed from production) |
| scenario_selector_editable_example.py | Usage example (has bugs, should be moved) |
| scenario_selector_editable_documentation.md | Documentation (should be moved to doc/) |
| IMPLEMENTATION_SUMMARY.md | Implementation notes (should be removed from production) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Scenario Selector - Editable Property | ||
|
|
||
| ## Overview | ||
|
|
||
| The `scenario_selector` visual element now supports an optional `editable` argument that controls whether users can edit scenario names and tags. | ||
|
|
||
| ## Property | ||
|
|
||
| ### editable | ||
|
|
||
| - **Type**: `bool` | ||
| - **Default**: `True` | ||
| - **Dynamic**: Yes (can be bound to state variables) | ||
|
|
||
| When `editable=False`: | ||
| - Users cannot edit scenario names | ||
| - Users cannot edit scenario tags | ||
| - The edit icon/button is hidden from the UI | ||
| - All other functionality remains available (selection, sorting, adding scenarios if `show_add_button=True`) | ||
|
|
||
| ## Usage Examples | ||
|
|
||
| ### Basic Usage - Disable Editing | ||
|
|
||
| ```python | ||
| from taipy.gui import Gui, Markdown | ||
|
|
||
| # Disable editing for all scenarios | ||
| page = Markdown(""" | ||
| <|scenario_selector|editable=False|> | ||
| """) | ||
|
|
||
| gui = Gui(page) | ||
| gui.run() | ||
| ``` | ||
|
|
||
| ### Dynamic Control with State Variable | ||
|
|
||
| ```python | ||
| from taipy.gui import Gui, Markdown, State | ||
|
|
||
| # Control editing through state variable | ||
| allow_editing = True | ||
|
|
||
| def toggle_editing(state: State): | ||
| state.allow_editing = not state.allow_editing | ||
|
|
||
| page = Markdown(""" | ||
| <|scenario_selector|editable={allow_editing}|> | ||
| <|Toggle Editing|button|on_action=toggle_editing|> | ||
| """) | ||
|
|
||
| gui = Gui(page) | ||
| gui.run() | ||
| ``` | ||
|
|
||
| ### Combined with Other Properties | ||
|
|
||
| ```python | ||
| from taipy.gui import Gui, Markdown | ||
|
|
||
| # Non-editable selector that still allows adding and searching | ||
| page = Markdown(""" | ||
| <|scenario_selector|editable=False|show_add_button=True|show_search=True|> | ||
| """) | ||
|
|
||
| gui = Gui(page) | ||
| gui.run() | ||
| ``` | ||
|
|
||
| ## Behavior | ||
|
|
||
| ### When editable=True (default) | ||
| - Edit icons appear next to each scenario | ||
| - Users can click edit icons to modify scenario names and tags | ||
| - Full editing functionality is available | ||
| - Maintains backward compatibility with existing code | ||
|
|
||
| ### When editable=False | ||
| - Edit icons are hidden from the UI | ||
| - Users cannot access scenario editing dialogs | ||
| - Selection, sorting, filtering, and searching still work normally | ||
| - Adding new scenarios still works if `show_add_button=True` | ||
|
|
||
| ## Backward Compatibility | ||
|
|
||
| This change is fully backward compatible. Existing `scenario_selector` implementations will continue to work exactly as before, with editing enabled by default. | ||
|
|
||
| ## Related Properties | ||
|
|
||
| The `editable` property works independently of other `scenario_selector` properties: | ||
|
|
||
| - `show_add_button`: Controls whether users can add new scenarios (independent of editing) | ||
| - `show_search`: Controls search functionality (independent of editing) | ||
| - `filter` and `sort`: Control filtering and sorting (independent of editing) | ||
| - `multiple`: Controls multi-selection (independent of editing) | ||
|
|
||
| ## Use Cases | ||
|
|
||
| 1. **Read-only dashboards**: Display scenarios without allowing modifications | ||
| 2. **Role-based access**: Disable editing for certain user roles while preserving other functionality | ||
| 3. **Approval workflows**: Show scenarios in read-only mode during approval processes | ||
| 4. **Audit views**: Display historical scenarios without edit capabilities No newline at end of file |
There was a problem hiding this comment.
This documentation file should be integrated into the official documentation structure (e.g., doc/ directory) instead of being in the root directory. Standalone markdown files in the root can clutter the repository structure.
| def on_scenario_change(state: State, var_name: str, value): | ||
| """Handle scenario selection changes.""" | ||
| state.selected_scenario = value | ||
| print(f"Selected scenario: {value.label if value else 'None'}") |
There was a problem hiding this comment.
The Scenario class does not have a label property. It has a get_label() method instead. This line should use value.get_label() instead of value.label.
| ## Current Settings | ||
| - **Editable**: {is_editable} | ||
| - **Selected Scenario**: {selected_scenario.label if selected_scenario else "None"} |
There was a problem hiding this comment.
The Scenario class does not have a label property. It has a get_label() method instead. This line should use selected_scenario.get_label() instead of selected_scenario.label.
| it("should still render add button when editable is false", () => { | ||
| render(<ScenarioSelector {...mockProps} editable={false} showAddButton={true} />); | ||
|
|
||
| // The add button should still be present | ||
| expect(screen.getByText("Add scenario")).toBeInTheDocument(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
This test will fail because the mock of CoreSelector doesn't actually render the "Add scenario" button. The button is rendered in the actual ScenarioSelector component (lines 532-546), not in CoreSelector. Since CoreSelector is mocked, the button won't be present in the DOM. This test should either be removed or the component structure should be tested differently.
| it("should still render add button when editable is false", () => { | |
| render(<ScenarioSelector {...mockProps} editable={false} showAddButton={true} />); | |
| // The add button should still be present | |
| expect(screen.getByText("Add scenario")).toBeInTheDocument(); | |
| }); |
| return False | ||
|
|
||
| if __name__ == "__main__": | ||
| success = validate_viselements_json() | ||
| sys.exit(0 if success else 1) No newline at end of file |
There was a problem hiding this comment.
This validation script should not be included in the production codebase. Validation scripts are typically temporary files used during development and testing. Consider removing this file before merging, or if it's needed for CI/CD, move it to a tools/ or scripts/ directory.
| return False | |
| if __name__ == "__main__": | |
| success = validate_viselements_json() | |
| sys.exit(0 if success else 1) | |
| return False |
| import pytest | ||
| from unittest.mock import Mock | ||
|
|
||
| from taipy.gui import Gui |
There was a problem hiding this comment.
Import of 'Gui' is not used.
| from taipy.gui import Gui |
| # an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations under the License. | ||
|
|
||
| import pytest |
There was a problem hiding this comment.
Import of 'pytest' is not used.
| import pytest |
| # specific language governing permissions and limitations under the License. | ||
|
|
||
| import pytest | ||
| from unittest.mock import Mock, patch |
There was a problem hiding this comment.
Import of 'Mock' is not used.
Import of 'patch' is not used.
| from unittest.mock import Mock, patch |
| import pytest | ||
| from unittest.mock import Mock, patch | ||
|
|
||
| from taipy.core import Scenario |
There was a problem hiding this comment.
Import of 'Scenario' is not used.
| from taipy.core import Scenario |
| from unittest.mock import Mock, patch | ||
|
|
||
| from taipy.core import Scenario | ||
| from taipy.gui import Gui, Markdown |
There was a problem hiding this comment.
Import of 'Markdown' is not used.
| from taipy.gui import Gui, Markdown | |
| from taipy.gui import Gui |
2 participants
This PR adds an optional
editable: bool = Trueargument toscenario_selector, allowing usersto disable editing of scenario names and tags when needed.
When
editable=False:This keeps default behavior unchanged while giving developers finer control over UI interactions.
Changes included:
editablebehaviorviselements.json)Backwards compatibility is preserved.
Fixes #2562