-
Notifications
You must be signed in to change notification settings - Fork 17
feat: adds global nodes management form #613
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
base: mrd/new-nodes-management-system
Are you sure you want to change the base?
feat: adds global nodes management form #613
Conversation
This commit addresses issue: OpenRailAssociation/osrd#13292 Details: - Adds new component GlobalNodesManagementComponent in app/view/editor-edit-tools-view-component - The new component controls the isCollapsed state of the nodes - Adds EN and FR translations for the new component - Adds the new component to the existing EditorEditToolsViewComponent panel
louisgreiner
left a comment
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.
Looks very nice and the search bar is really good.
I wonder if we could ask user's confirmation when checking the box for all the nodes at once? This component already exists in the application I think, we can reuse it.
PS: you should make your PR into mrd/new-nodes-management-system I think, we use this as feature branch
...ditor-edit-tools-view-component/global-nodes-management/global-nodes-management.component.ts
Outdated
Show resolved
Hide resolved
src/assets/i18n/fr.json
Outdated
| "nodes": "Noeuds" | ||
| } | ||
| }, | ||
| "nodes-search-placeholder": "Rechercher par noms ou trigrammes", |
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.
| "nodes-search-placeholder": "Rechercher par noms ou trigrammes", | |
| "nodes-search-placeholder": "Rechercher par nom ou trigramme", |
| /> | ||
| </div> | ||
| <div class="nodes-list"> | ||
| @if (getVisibleNodes().length) { |
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.
Hum I've not worked with Angular for a while, but why not use a useMemo for visibleNodes ?
This commit handles various feedbacks from reviewers on PR: #613 Details: - Fixes some FR translations - Renames "visible nodes" as "matching nodes" - Caches the matching nodes array - Adds a confirmation modal when clicking the global checkbox
Description
This PR depends on PR #554. It should only be merged once #554 is merged as well.
This PR adds a new block in the "Edit" side panel, to allow controlling the
isCollapsedstate of the nodes.This form displays:
isCollapsedstateIssues
Checklist
documentation/