Improve clones form#2342
Open
Yago004 wants to merge 12 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the clones management UI to hide the “Clones” settings tab when a base machine has no clones, and adds a confirmation step when starting clones from the base machine’s clones form.
Changes:
- Gate the “Clones” settings tab behind
$domain->has_clonesfor base machines. - Add a start-clones confirmation panel (and improved styling) in the clones form.
- Add new i18n strings for “Yes, start all clones” (EN/ES/CA) and adjust CA “Cancel” capitalization.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/main/vm_clones.html.ep | Adds a start confirmation UI and improves shutdown confirmation styling. |
| templates/main/settings_machine_tabs_head.html.ep | Hides the “Clones” tab when a base has no clones. |
| lib/Ravada/I18N/en.po | Adds translation entry for the new start confirmation label. |
| lib/Ravada/I18N/es.po | Adds Spanish translation for the new start confirmation label. |
| lib/Ravada/I18N/ca.po | Adds Catalan translation for the new start confirmation label and capitalizes “Cancel”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
35
to
36
| % if ($USER->is_admin && $domain->is_base && $domain->has_clones){ | ||
| <a class="nav-link" id="v-pills-remove-tab" href="#v-pills-clones" data-toggle="pill" role="tab" aria-controls="v-pills-clones" aria-selected="false"><%=l 'Clones' %></a> |
Comment on lines
+34
to
+40
| <a type="button" class="btn btn-secondary text-white mr-2" | ||
| ng-click="start_clones_prompt=0" | ||
| ><%=l 'Cancel' %></a> | ||
|
|
||
| <a type="button" class="btn btn-primary text-white" | ||
| ng-click="request('start_clones',{ 'id_domain': showmachine.id, 'at': isPastTime(exec_time) ? '' : getUnixTimeFromDate(exec_time), 'exec_sequentially': exec_sequentially }); start_clones_prompt=0" | ||
| ><%=l 'Yes, start all clones' %></a> |
Comment on lines
+2593
to
+2594
| msgid "Yes, start all clones" | ||
| msgstr "Yes, start all clones" |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
templates/main/vm_clones.html.ep:52
- The confirmation label has a typo: "shutwdown". Because this string is used as an i18n key (via l '...'), fixing it correctly requires updating translation entries (and possibly msgids) across locales to avoid regressions.
<a type="button" class="btn btn-danger text-white" ng-click="request('shutdown_clones',{ 'id_domain': showmachine.id, 'at': isPastTime(exec_time) ? '' : getUnixTimeFromDate(exec_time) });shutdown_clones=0"
><%=l 'Yes, shutwdown all the clones' %></a>
Comment on lines
+35
to
+36
| % if ($USER->is_admin && $domain->is_base){ | ||
| <a class="nav-link" id="v-pills-remove-tab" href="#v-pills-clones" data-toggle="pill" role="tab" aria-controls="v-pills-clones" aria-selected="false"><%=l 'Clones' %></a> | ||
| <a class="nav-link" id="v-pills-remove-tab" ng-show="showmachine.has_clones" href="#v-pills-clones" data-toggle="pill" role="tab" aria-controls="v-pills-clones" aria-selected="false"><%=l 'Clones' %></a> |
| ><%=l 'Bundle' %></a> | ||
| % } | ||
| </div> | ||
| </div> |
| ng-click="start_clones_prompt=0" | ||
| ><%=l 'Cancel' %></button> | ||
|
|
||
| <button type="button" class="btn btn-primary text-white" |
UPC#2347) Update messages.pot and add test to verify it contains all po strings --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
templates/main/vm_clones.html.ep:52
- The shutdown confirmation uses
<a>elements withouthrefas buttons, which is not great for accessibility and also makes it hard to enforce a disabled state. Consider using real<button type="button">elements and disable the destructive action whenexec_timeis in the past (to avoid unintentionally running immediately).
<a type="button" class="btn btn-primary text-white mr-2"
ng-click="shutdown_clones=0"
><%=l 'No' %></a>
<a type="button" class="btn btn-danger text-white" ng-click="request('shutdown_clones',{ 'id_domain': showmachine.id, 'at': isPastTime(exec_time) ? '' : getUnixTimeFromDate(exec_time) });shutdown_clones=0"
><%=l 'Yes, shutwdown all the clones' %></a>
Comment on lines
+12
to
+20
| <button class="btn btn-primary mr-2" id="start-clones" name="start-clones" | ||
| ng-disabled="isPastTime(exec_time, exec_time_start)" | ||
| ng-click="request('start_clones',{ 'id_domain': showmachine.id, 'at': isPastTime(exec_time) ? '' : getUnixTimeFromDate(exec_time), 'exec_sequentially': exec_sequentially })" | ||
| ng-click="start_clones_prompt=1; shutdown_clones=0" | ||
| > | ||
| <%=l 'Start all clones' %> | ||
| </button> | ||
| <button class="btn btn-danger" id="shutdown-clones" name="shutdown-clones" | ||
| ng-disabled="isPastTime(exec_time, exec_time_start)" | ||
| ng-click="shutdown_clones=1" | ||
| ng-click="shutdown_clones=1; start_clones_prompt=0" |
Comment on lines
+38
to
+40
| <button type="button" class="btn btn-primary text-white" | ||
| ng-click="request('start_clones',{ 'id_domain': showmachine.id, 'at': isPastTime(exec_time) ? '' : getUnixTimeFromDate(exec_time), 'exec_sequentially': exec_sequentially }); start_clones_prompt=0" | ||
| ><%=l 'Yes, start all clones' %></button> |
Improved radio button interaction issue UPC#2341
…mprove_clonesform
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removed 'Clones' row from machine settings when machine is base but has no clones.
Improved the form to close and open clones from base.