Merge pull request #30 from A2-ai/modal_mods

Modal mods

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: ghqc.app
 Title: Create QC Checklists in Github Issues
-Version: 0.1.8
+Version: 0.1.9
 Authors@R: c(
     person("Jenna", "Johnson", email = "[email protected]", role = c("aut", "cre")),
     person("Anne", "Zheng", email = "[email protected]", role = c("aut")),

NEWS.md

@@ -1,3 +1,7 @@
+# ghqc.app 0.1.9
+
+- Clarifies language in Warning and Error modals for all three apps
+
 # ghqc.app 0.1.8 
 
 - Removes reset button in record app modal for users to click after closing milestones when no closed milestones exist (was buggy in viewer panel)

R/app_01_assign_fct_treeNavigator.R

@@ -9,7 +9,7 @@ NULL
 #' Generate Excluded File Message
 #'
 #' This function generates an HTML formatted message indicating which files are excluded
-#' from selection as QC items in a directory.
+#' from selection as Issues in a directory.
 #'
 #' @param excluded_files A character vector of file paths that are excluded from selection.
 #'

R/app_01_assign_server.R

@@ -286,20 +286,35 @@ return "<div><strong>" + escape(item.username) + "</div>"
 
     observeEvent(input$create_qc_items, {
       req(modal_check())
-
       if (!is.null(modal_check()$message)) {
-        showModal(modalDialog(
-          title = tags$div(tagList(
-            if (modal_check()$state == "warning") {
-              actionButton(ns("proceed"), "Proceed Anyway")
-            },
-            actionButton(ns("return"), "Return")
-          ), style = "text-align: right;"),
-          HTML(modal_check()$message),
-          footer = NULL,
-          easyClose = TRUE
-        ))
-      } else {
+        if (modal_check()$state == "warning") {
+          showModal(modalDialog(
+            title = tags$div(
+              tags$span("Warning", style = "float: left; font-weight: bold; font-size: 20px; margin-top: 5px;"),
+              actionButton(ns("proceed"), "Proceed Anyway"),
+              actionButton(ns("return"), "Return"),
+              style = "text-align: right;"
+            ),
+            HTML(modal_check()$message),
+            footer = NULL,
+            easyClose = TRUE
+          ))
+        }
+
+        else if (modal_check()$state == "error") {
+          showModal(modalDialog(
+            title = tags$div(
+              tags$span("Error", style = "float: left; font-weight: bold; font-size: 20px; margin-top: 5px;"),
+              actionButton(ns("return"), "Return"),
+              style = "text-align: right;"
+            ),
+            HTML(modal_check()$message),
+            footer = NULL,
+            easyClose = TRUE
+          ))
+        }
+      }
+      else {
         qc_trigger(TRUE)
       }
     })
@@ -437,7 +452,7 @@ return "<div><strong>" + escape(item.username) + "</div>"
     })
 
     observeEvent(input$return, {
-      debug(.le$logger, glue::glue("Create QC items action returned and modal removed."))
+      debug(.le$logger, glue::glue("Assign action returned and modal removed."))
       removeModal()
     })
 

R/app_02_resolve_server.R

@@ -163,40 +163,71 @@ ghqc_resolve_server <- function(id, remote, org, repo, milestone_list) {
 
     observeEvent(input$preview, {
       req(modal_check())
-
+      req(modal_check())
       if (!is.null(modal_check()$message)) {
-        showModal(modalDialog(
-          title = tags$div(tagList(
-            if (modal_check()$state == "warning") {
-              actionButton(ns("proceed_preview"), "Proceed Anyway")
-            },
-            actionButton(ns("return"), "Return")
-          ), style = "text-align: right;"),
-          HTML(modal_check()$message),
-          footer = NULL,
-          easyClose = TRUE
-        ))
-      } else {
+
+        if (modal_check()$state == "warning") {
+          showModal(modalDialog(
+            title = tags$div(
+              tags$span("Warning", style = "float: left; font-weight: bold; font-size: 20px; margin-top: 5px;"),
+              actionButton(ns("proceed_preview"), "Proceed Anyway"),
+              actionButton(ns("return"), "Return"),
+              style = "text-align: right;"
+            ),
+            HTML(modal_check()$message),
+            footer = NULL,
+            easyClose = TRUE
+          ))
+        }
+        else if (modal_check()$state == "error") {
+          showModal(modalDialog(
+            title = tags$div(
+              tags$span("Error", style = "float: left; font-weight: bold; font-size: 20px; margin-top: 5px;"),
+              actionButton(ns("return"), "Return"),
+              style = "text-align: right;"
+            ),
+            HTML(modal_check()$message),
+            footer = NULL,
+            easyClose = TRUE
+          ))
+        }
+      }
+      else {
         preview_trigger(TRUE)
       }
     })
 
     observeEvent(input$post, {
       req(modal_check())
-
       if (!is.null(modal_check()$message)) {
-        showModal(modalDialog(
-          title = tags$div(tagList(
-            if (modal_check()$state == "warning") {
-              actionButton(ns("proceed_post"), "Proceed Anyway")
-            },
-            actionButton(ns("return"), "Return")
-          ), style = "text-align: right;"),
-          HTML(modal_check()$message),
-          footer = NULL,
-          easyClose = TRUE
-        ))
-      } else {
+        if (modal_check()$state == "warning") {
+          showModal(modalDialog(
+            title = tags$div(
+                  tags$span("Warning", style = "float: left; font-weight: bold; font-size: 20px; margin-top: 5px;"),
+                  actionButton(ns("proceed_post"), "Proceed Anyway"),
+                  actionButton(ns("return"), "Return"),
+                  style = "text-align: right;"
+                  ),
+            HTML(modal_check()$message),
+            footer = NULL,
+            easyClose = TRUE
+          ))
+        }
+
+        else if (modal_check()$state == "error") {
+          showModal(modalDialog(
+            title = tags$div(
+              tags$span("Error", style = "float: left; font-weight: bold; font-size: 20px; margin-top: 5px;"),
+              actionButton(ns("return"), "Return"),
+              style = "text-align: right;"
+            ),
+            HTML(modal_check()$message),
+            footer = NULL,
+            easyClose = TRUE
+          ))
+        }
+      }
+      else {
         post_trigger(TRUE)
       }
     })

R/app_03_record_server.R

@@ -32,9 +32,8 @@ ghqc_record_server <- function(id, remote, org, repo, all_milestones) {
             showModal(
               modalDialog(
                 title = tags$div(
-                  tags$span("Warning", style = "float: left; font-weight: bold; font-size: 20px;"),
+                  tags$span("Warning", style = "float: left; font-weight: bold; font-size: 20px; margin-top: 5px;"),
                   tags$div(modalButton("Dismiss"), style = "text-align: right;"),
-                  #actionButton(ns("return"), "Return", class = "btn-sm"),
                   style = "#overflow: hidden; text-align: right;"
                 ),
 
@@ -110,32 +109,32 @@ ghqc_record_server <- function(id, remote, org, repo, all_milestones) {
 
     observeEvent(input$generate_report, {
       req(modal_check())
-
       if (!is.null(modal_check()$message)) {
-        showModal(modalDialog(
-          title = tags$div(tagList(
-            if (modal_check()$state == "warning") {
-              tags$span("Warning", style = "float: left; font-weight: bold; font-size: 20px;")
-              actionButton(ns("proceed"), "Proceed Anyway")
-            },
-            actionButton(ns("return"), "Return")
-          ), style = "text-align: right;"),
-          HTML(paste("It is recommended that relevant GitHub Issues and Milestones are closed upon completion of QC, and checklists within GitHub Issues are checked to indicate QCed items.<br><br> You may want to double check the following items for outstanding QC progress:<br><br>", modal_check()$message)),
-          tags$style(HTML("
+        if (modal_check()$state == "warning") {
+          showModal(modalDialog(
+            title = tags$div(
+              tags$span("Warning", style = "float: left; font-weight: bold; font-size: 20px; margin-top: 5px;"),
+              actionButton(ns("proceed"), "Proceed Anyway"),
+              actionButton(ns("return"), "Return"),
+              style = "text-align: right;"
+            ),
+            HTML(paste("It is recommended that relevant GitHub Issues and Milestones are closed upon completion of QC, and checklists within GitHub Issues are checked to indicate QCed items.<br><br> You may want to double check the following items for outstanding QC progress:<br><br>", modal_check()$message)),
+            tags$style(HTML("
         .modal-content {
           word-wrap: break-word; /* Allows long text to break into new lines */
           overflow-wrap: break-word;
           max-width: 100%; /* Ensures content fits within the modal */
         }
       ")),
-          footer = NULL,
-          easyClose = TRUE
-        ))
+            footer = NULL,
+            easyClose = TRUE
+          ))
+        }
       }
       else {
         report_trigger(TRUE)
       }
-    })
+     })
 
     observe({
       req(report_trigger())
@@ -197,13 +196,13 @@ ghqc_record_server <- function(id, remote, org, repo, all_milestones) {
     })
 
     observeEvent(input$proceed, {
-      debug(.le$logger, glue::glue("Report QC items action proceeded and modal removed."))
+      debug(.le$logger, glue::glue("Generate QC Record action proceeded and modal removed."))
       removeModal()
       report_trigger(TRUE)
     })
 
     observeEvent(input$return, {
-      debug(.le$logger, glue::glue("Report QC items action returned and modal removed."))
+      debug(.le$logger, glue::glue("Generate QC Record action returned and modal removed."))
       removeModal()
     })
 

R/app_fct_determine_modal_message.R

@@ -10,7 +10,7 @@ generate_sync_message <- function(git_sync_status, error_icon_html) {
     sync_messages <- c()
     if (git_sync_status$ahead > 0) sync_messages <- c(sync_messages, "push changes to the remote repository")
     if (git_sync_status$behind > 0) sync_messages <- c(sync_messages, "pull updates from the remote repository")
-    messages <- paste(error_icon_html, "There are local changes that need to be synchronized. Please", paste(sync_messages, collapse = " and "), "<br>")
+    messages <- paste(error_icon_html, "There are repository changes that need to be synchronized. Please", paste(sync_messages, collapse = " and "), "<br>")
   }
   return(messages)
 }
@@ -20,13 +20,13 @@ generate_uncommitted_message <- function(uncommitted_files, error_icon_html, war
   messages <- c()
   if (length(uncommitted_files$selected) > 0) {
     messages <- c(messages, sprintf(
-      "%s All files to be QC'd must have any local changes committed before proceeding. The following selected local files have uncommitted changes:<ul>%s</ul><br>",
+      "%s All selected files must have local changes committed before proceeding. The following selected files have local uncommitted changes:<br><br><ul>%s</ul><br>",
       error_icon_html, generate_html_list(uncommitted_files$selected)
     ))
   }
   if (length(uncommitted_files$general) > 0 && length(uncommitted_files$selected) == 0) {
     messages <- c(messages, sprintf(
-      "%s There are local files, which are not in the selected QC items, that have uncommitted changes:<ul>%s</ul><br>",
+      "%s The following local files have uncommitted changes, but are not selected:<ul>%s</ul><br>",
       warning_icon_html, generate_html_list(uncommitted_files$general)
     ))
   }
@@ -50,7 +50,7 @@ generate_commit_update_message <- function(commit_update_status, error_icon_html
   messages <- c()
 
   if (!commit_update_status) {
-    messages <- c(messages, paste(error_icon_html, "There are no update commits on the QC item since QC initialization.<br>"))
+    messages <- c(messages, paste(error_icon_html, "There are no new commits since QC initialization.<br>"))
   }
 
   return(messages)
@@ -87,10 +87,22 @@ determine_modal_message <- function(selected_files,
   existing_issues <- selected_files[selected_files %in% issue_titles]
 
   messages <- c()
-  messages <- c(messages, generate_sync_message(git_sync_status, error_icon_html))
-  messages <- c(messages, generate_uncommitted_message(uncommitted_files, error_icon_html, warning_icon_html))
+
+  # Errors
+  sync_message <- generate_sync_message(git_sync_status, error_icon_html)
+  messages <- c(messages, sync_message)
+  if (length(sync_message) == 0) {
+    # In the case that there are remote changes, saying that there are no new commits just isn't accurate
+    # (there are no new local commits, but there are remote commits in this case)
+    # So only give the no new commits message if there aren't any unsynced commits
+    messages <- c(messages, generate_commit_update_message(commit_update_status, error_icon_html))
+  }
   messages <- c(messages, generate_existing_issue_message(existing_issues, error_icon_html))
-  messages <- c(messages, generate_commit_update_message(commit_update_status, error_icon_html))
+
+  # Errors and Warnings
+  messages <- c(messages, generate_uncommitted_message(uncommitted_files, error_icon_html, warning_icon_html))
+
+
 
   log_string <- glue::glue("Modal Check Inputs:
     - Selected Files: {glue::glue_collapse(selected_files, sep = ', ')}

tests/testthat/test-determine_modal_message.R

@@ -19,7 +19,7 @@ test_that("determine_modal_message works correctly if there are messages", {
   expect_true(grepl("file1.txt", result$message))
   expect_true(grepl("file2.txt", result$message))
   expect_true(grepl("file1.txt", result$message))
-  expect_true(grepl("There are no update commits on the QC item", result$message))
+  #expect_true(grepl("There are no new commits since QC initialization.", result$message))
   expect_equal(result$state, "error")
 })
 
@@ -55,7 +55,7 @@ test_that("generate_uncommitted_message works correctly for uncommitted general
   result <- generate_uncommitted_message(uncommitted_files, error_icon_html, warning_icon_html)
 
   expect_true(length(result) == 1)
-  expect_true(grepl("There are local files, which are not in the selected QC items, that have uncommitted changes", result))
+  expect_true(grepl("The following local files have uncommitted changes, but are not selected", result))
   expect_true(grepl("file4.txt", result))
   expect_true(grepl("file5.txt", result))
   expect_true(grepl("<warning>", result))