From 5bbdea2875d64e0f513ffea354d9d60be91ed7d4 Mon Sep 17 00:00:00 2001 From: Kamil Foltynski Date: Fri, 8 Aug 2025 15:32:44 +0200 Subject: [PATCH 1/3] Fix modal not closing when using multiple shinyDirChoose buttons This commit fixes the issue where modals don't close after clicking 'Select' when using multiple shinyDirChoose buttons with different IDs in R Shiny apps. Root Cause: - Each shinyDirChoose button creates its own modal with unique ID - JavaScript event handlers used generic selectors (.sF-modalContainer) that could match multiple modals - When multiple modals exist, event handlers may target wrong modal or cause conflicts - No cleanup of existing modals when creating new ones Changes Made: 1. Added modal cleanup on creation: - Added cleanup code in createFileChooser(), createDirChooser(), createFileSaver() - Removes existing modals for same button before creating new one - Ensures each button only has one active modal at a time 2. Fixed keydown handler: - Changed from generic selector to finding currently visible modal - Uses .sF-modalContainer:visible.first() instead of .sF-modalContainer - Prevents conflicts between multiple modals 3. Fixed backdrop click handler: - Updated to work with specific modal that was clicked - Uses .find() instead of global selectors - Ensures correct modal is closed when clicking backdrop 4. Fixed escape key handler: - Updated to work with currently visible modal - Uses visibleModal.find() instead of global selectors - Ensures escape key closes correct modal Impact: - Resolves modal not closing issue with multiple shinyDirChoose buttons - Backward compatible - doesn't affect single modal usage - Improves reliability when using multiple file/directory choosers --- FIX_MULTIPLE_MODALS.md | 106 +++++++++++++++++++++++++++++++++++++++++ inst/www/shinyFiles.js | 64 ++++++++++++++++--------- test_multiple_dirs.R | 63 ++++++++++++++++++++++++ 3 files changed, 211 insertions(+), 22 deletions(-) create mode 100644 FIX_MULTIPLE_MODALS.md create mode 100644 test_multiple_dirs.R diff --git a/FIX_MULTIPLE_MODALS.md b/FIX_MULTIPLE_MODALS.md new file mode 100644 index 0000000..e580f22 --- /dev/null +++ b/FIX_MULTIPLE_MODALS.md @@ -0,0 +1,106 @@ +# Fix for Multiple shinyDirChoose Modal Issue + +## Problem Description + +When using multiple `shinyDirChoose` buttons with different IDs in an R Shiny app, the modal where directories are selected does not close after clicking "Select". This issue occurs because: + +1. Each `shinyDirChoose` button creates its own modal with a unique ID +2. The JavaScript event handlers use generic selectors (`.sF-modalContainer`) that can match multiple modals +3. When multiple modals exist, the event handlers may target the wrong modal or cause conflicts + +## Root Cause + +The issue was in the JavaScript code in `inst/www/shinyFiles.js`: + +1. **No cleanup of existing modals**: When creating a new modal, existing modals for the same button were not cleaned up +2. **Generic selectors in event handlers**: The keydown handler and backdrop click handler used generic selectors that could match multiple modals +3. **Event handler conflicts**: Multiple modals could interfere with each other's event handling + +## Solution + +The fix includes several changes to `inst/www/shinyFiles.js`: + +### 1. Modal Cleanup on Creation + +Added cleanup of existing modals when creating new ones in all three modal creation functions: + +```javascript +// Clean up any existing modals for this button +var existingModal = $(button).data('modal'); +if (existingModal) { + removeFileChooser(button, existingModal); +} +``` + +This ensures that each button only has one active modal at a time. + +### 2. Fixed Keydown Handler + +Changed the keydown handler to work with multiple modals by finding the currently visible modal: + +```javascript +// Before (problematic) +if ($(".sF-modalContainer").is(":visible")) { + var modalButton = $($(".sF-modalContainer").data('button')); + // ... +} + +// After (fixed) +var visibleModal = $(".sF-modalContainer:visible").first(); +if (visibleModal.length > 0) { + var modalButton = visibleModal.data('button'); + // ... +} +``` + +### 3. Fixed Backdrop Click Handler + +Updated the backdrop click handler to work with the specific modal that was clicked: + +```javascript +// Before (problematic) +if (!$(e.target).closest('.modal-content').length > 0 && $("#sF-cancelButton").is(":visible")) { + $("#sF-cancelButton").click(); +} + +// After (fixed) +if (!$(e.target).closest('.modal-content').length > 0 && $(this).find("#sF-cancelButton").is(":visible")) { + $(this).find("#sF-cancelButton").click(); +} +``` + +### 4. Fixed Escape Key Handler + +Updated the escape key handler to work with the currently visible modal: + +```javascript +// Before (problematic) +if ($("#sF-cancelButton").is(":visible") && !$("div.sF-newDir").hasClass("open")) { + $("#sF-cancelButton").click(); +} + +// After (fixed) +var visibleModal = $(".sF-modalContainer:visible").first(); +if (visibleModal.length > 0 && visibleModal.find("#sF-cancelButton").is(":visible") && !visibleModal.find("div.sF-newDir").hasClass("open")) { + visibleModal.find("#sF-cancelButton").click(); +} +``` + +## Testing + +A test app (`test_multiple_dirs.R`) has been created to verify the fix works correctly. The test app includes three `shinyDirChoose` buttons to ensure that: + +1. Each modal opens and closes correctly +2. The "Select" button properly closes the modal +3. Multiple modals don't interfere with each other +4. Keyboard shortcuts (Enter, Escape) work correctly + +## Files Modified + +- `inst/www/shinyFiles.js`: Fixed modal creation and event handlers +- `test_multiple_dirs.R`: Test app to verify the fix +- `FIX_MULTIPLE_MODALS.md`: This documentation + +## Impact + +This fix resolves the issue where modals don't close after clicking "Select" when using multiple `shinyDirChoose` buttons. The fix is backward compatible and doesn't affect single modal usage. diff --git a/inst/www/shinyFiles.js b/inst/www/shinyFiles.js index f039098..6262070 100644 --- a/inst/www/shinyFiles.js +++ b/inst/www/shinyFiles.js @@ -792,6 +792,12 @@ var shinyFiles = (function () { // File chooser var createFileChooser = function (button, title) { + // Clean up any existing modals for this button + var existingModal = $(button).data('modal'); + if (existingModal) { + removeFileChooser(button, existingModal); + } + // Preparations $(button).prop('disabled', true); @@ -1351,6 +1357,12 @@ var shinyFiles = (function () { // File saver var createFileSaver = function (button, title) { + // Clean up any existing modals for this button + var existingModal = $(button).data('modal'); + if (existingModal) { + removeFileChooser(button, existingModal); + } + // Preparations $(button).prop('disabled', true); @@ -1827,6 +1839,12 @@ var shinyFiles = (function () { // Directory chooser var createDirChooser = function (button, title) { + // Clean up any existing modals for this button + var existingModal = $(button).data('modal'); + if (existingModal) { + removeFileChooser(button, existingModal); + } + // Preparations $(button).prop('disabled', true); @@ -2598,8 +2616,9 @@ var shinyFiles = (function () { switch (event.keyCode) { case 27: // Escape - if ($("#sF-cancelButton").is(":visible") && !$("div.sF-newDir").hasClass("open")) { - $("#sF-cancelButton").click(); + var visibleModal = $(".sF-modalContainer:visible").first(); + if (visibleModal.length > 0 && visibleModal.find("#sF-cancelButton").is(":visible") && !visibleModal.find("div.sF-newDir").hasClass("open")) { + visibleModal.find("#sF-cancelButton").click(); }; break; @@ -2622,38 +2641,39 @@ var shinyFiles = (function () { case 13: // Enter - if ($(".sF-modalContainer").is(":visible")) { - var modalButton = $($(".sF-modalContainer").data('button')); - var lastElement = $(".sF-fileList").data('lastElement'); + var visibleModal = $(".sF-modalContainer:visible").first(); + if (visibleModal.length > 0) { + var modalButton = visibleModal.data('button'); + var lastElement = visibleModal.find(".sF-fileList").data('lastElement'); if (modalButton.hasClass("shinyFiles")) { - if (!$($(".sF-fileList").data('lastElement')).hasClass('selected')) { return; } + if (!lastElement || !$(lastElement).hasClass('selected')) { return; } // Select File - if ($($(".sF-fileList").data('lastElement')).hasClass('sF-file')) { - selectFiles(modalButton, $(".sF-modalContainer")); - } else if ($($(".sF-fileList").data('lastElement')).hasClass('sF-directory')) { - openDir(modalButton, $(".sF-modalContainer"), $($(".sF-fileList").data('lastElement'))); + if ($(lastElement).hasClass('sF-file')) { + selectFiles(modalButton, visibleModal); + } else if ($(lastElement).hasClass('sF-directory')) { + openDir(modalButton, visibleModal, $(lastElement)); } } else if (modalButton.hasClass("shinySave")) { // Assume the button is properly disabled/enabled - if ($('.sF-filename').is(":focus") || $($(".sF-fileList").data('lastElement')).hasClass('sF-file')) { - var filename = $(".sF-filename").val(); + if (visibleModal.find('.sF-filename').is(":focus") || (lastElement && $(lastElement).hasClass('sF-file'))) { + var filename = visibleModal.find(".sF-filename").val(); var parts = filename.split("."); - if ($("#sF-selectButton").prop('disabled')) { return; } + if (visibleModal.find("#sF-selectButton").prop('disabled')) { return; } // Do not use enter to submit an empty filename (just a file extension) - if (($(".sF-filetype").length > 0 && filename.length > 0) || parts.slice(0, parts.length - 1).join(".").length > 0) { - saveFile($('.sF-modalContainer'), modalButton); + if ((visibleModal.find(".sF-filetype").length > 0 && filename.length > 0) || parts.slice(0, parts.length - 1).join(".").length > 0) { + saveFile(visibleModal, modalButton); } - } else if ($($(".sF-fileList").data('lastElement')).hasClass('sF-directory')) { - openDir(modalButton, $(".sF-modalContainer"), $($(".sF-fileList").data('lastElement'))); + } else if (lastElement && $(lastElement).hasClass('sF-directory')) { + openDir(modalButton, visibleModal, $(lastElement)); } } else if (modalButton.hasClass("shinyDirectories")) { - // Save File - if ($($(".sF-dirList").find(".selected")).length === 1) { - selectFiles(modalButton, $(".sF-modalContainer")); + // Directory selection + if (visibleModal.find(".sF-dirList .selected").length === 1) { + selectFiles(modalButton, visibleModal); } } } @@ -2662,8 +2682,8 @@ var shinyFiles = (function () { // Close modal when clicking on backdrop $(document).on('click', '.sF-modalContainer', function (e) { - if (!$(e.target).closest('.modal-content').length > 0 && $("#sF-cancelButton").is(":visible")) { - $("#sF-cancelButton").click(); + if (!$(e.target).closest('.modal-content').length > 0 && $(this).find("#sF-cancelButton").is(":visible")) { + $(this).find("#sF-cancelButton").click(); } }); }; diff --git a/test_multiple_dirs.R b/test_multiple_dirs.R new file mode 100644 index 0000000..bdc7092 --- /dev/null +++ b/test_multiple_dirs.R @@ -0,0 +1,63 @@ +library(shiny) +library(shinyFiles) +library(fs) + +ui <- fluidPage( + titlePanel("Test Multiple Directory Choosers"), + sidebarLayout( + sidebarPanel( + h4("Multiple Directory Choosers"), + p("This test app has multiple directory choosers to verify that modals close correctly."), + hr(), + shinyDirButton("dir1", "Directory 1", "Please select first directory"), + br(), br(), + shinyDirButton("dir2", "Directory 2", "Please select second directory"), + br(), br(), + shinyDirButton("dir3", "Directory 3", "Please select third directory"), + hr(), + p("Try opening multiple directory choosers and clicking 'Select' - the modal should close properly.") + ), + mainPanel( + h4("Selected Directories:"), + verbatimTextOutput("dir1_output"), + verbatimTextOutput("dir2_output"), + verbatimTextOutput("dir3_output") + ) + ) +) + +server <- function(input, output, session) { + volumes <- c(Home = fs::path_home(), "R Installation" = R.home(), getVolumes()()) + + # Set up multiple directory choosers + shinyDirChoose(input, "dir1", roots = volumes, session = session) + shinyDirChoose(input, "dir2", roots = volumes, session = session) + shinyDirChoose(input, "dir3", roots = volumes, session = session) + + # Output the selected directories + output$dir1_output <- renderPrint({ + if (is.integer(input$dir1)) { + cat("Directory 1: No directory selected") + } else { + cat("Directory 1:", parseDirPath(volumes, input$dir1)) + } + }) + + output$dir2_output <- renderPrint({ + if (is.integer(input$dir2)) { + cat("Directory 2: No directory selected") + } else { + cat("Directory 2:", parseDirPath(volumes, input$dir2)) + } + }) + + output$dir3_output <- renderPrint({ + if (is.integer(input$dir3)) { + cat("Directory 3: No directory selected") + } else { + cat("Directory 3:", parseDirPath(volumes, input$dir3)) + } + }) +} + +shinyApp(ui = ui, server = server) From f67902e236fedcb05f6f46b3832f0dabae4ec099 Mon Sep 17 00:00:00 2001 From: Kamil Foltynski Date: Fri, 8 Aug 2025 15:33:12 +0200 Subject: [PATCH 2/3] version bump --- DESCRIPTION | 2 +- FIX_MULTIPLE_MODALS.md | 106 ----------------------------------------- test_multiple_dirs.R | 63 ------------------------ 3 files changed, 1 insertion(+), 170 deletions(-) delete mode 100644 FIX_MULTIPLE_MODALS.md delete mode 100644 test_multiple_dirs.R diff --git a/DESCRIPTION b/DESCRIPTION index ac4bcc2..ae26768 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: shinyFiles Type: Package Title: A Server-Side File System Viewer for Shiny -Version: 0.9.3.9000 +Version: 0.9.4.9000 Authors@R: c(person(given = "Thomas Lin", family = "Pedersen", diff --git a/FIX_MULTIPLE_MODALS.md b/FIX_MULTIPLE_MODALS.md deleted file mode 100644 index e580f22..0000000 --- a/FIX_MULTIPLE_MODALS.md +++ /dev/null @@ -1,106 +0,0 @@ -# Fix for Multiple shinyDirChoose Modal Issue - -## Problem Description - -When using multiple `shinyDirChoose` buttons with different IDs in an R Shiny app, the modal where directories are selected does not close after clicking "Select". This issue occurs because: - -1. Each `shinyDirChoose` button creates its own modal with a unique ID -2. The JavaScript event handlers use generic selectors (`.sF-modalContainer`) that can match multiple modals -3. When multiple modals exist, the event handlers may target the wrong modal or cause conflicts - -## Root Cause - -The issue was in the JavaScript code in `inst/www/shinyFiles.js`: - -1. **No cleanup of existing modals**: When creating a new modal, existing modals for the same button were not cleaned up -2. **Generic selectors in event handlers**: The keydown handler and backdrop click handler used generic selectors that could match multiple modals -3. **Event handler conflicts**: Multiple modals could interfere with each other's event handling - -## Solution - -The fix includes several changes to `inst/www/shinyFiles.js`: - -### 1. Modal Cleanup on Creation - -Added cleanup of existing modals when creating new ones in all three modal creation functions: - -```javascript -// Clean up any existing modals for this button -var existingModal = $(button).data('modal'); -if (existingModal) { - removeFileChooser(button, existingModal); -} -``` - -This ensures that each button only has one active modal at a time. - -### 2. Fixed Keydown Handler - -Changed the keydown handler to work with multiple modals by finding the currently visible modal: - -```javascript -// Before (problematic) -if ($(".sF-modalContainer").is(":visible")) { - var modalButton = $($(".sF-modalContainer").data('button')); - // ... -} - -// After (fixed) -var visibleModal = $(".sF-modalContainer:visible").first(); -if (visibleModal.length > 0) { - var modalButton = visibleModal.data('button'); - // ... -} -``` - -### 3. Fixed Backdrop Click Handler - -Updated the backdrop click handler to work with the specific modal that was clicked: - -```javascript -// Before (problematic) -if (!$(e.target).closest('.modal-content').length > 0 && $("#sF-cancelButton").is(":visible")) { - $("#sF-cancelButton").click(); -} - -// After (fixed) -if (!$(e.target).closest('.modal-content').length > 0 && $(this).find("#sF-cancelButton").is(":visible")) { - $(this).find("#sF-cancelButton").click(); -} -``` - -### 4. Fixed Escape Key Handler - -Updated the escape key handler to work with the currently visible modal: - -```javascript -// Before (problematic) -if ($("#sF-cancelButton").is(":visible") && !$("div.sF-newDir").hasClass("open")) { - $("#sF-cancelButton").click(); -} - -// After (fixed) -var visibleModal = $(".sF-modalContainer:visible").first(); -if (visibleModal.length > 0 && visibleModal.find("#sF-cancelButton").is(":visible") && !visibleModal.find("div.sF-newDir").hasClass("open")) { - visibleModal.find("#sF-cancelButton").click(); -} -``` - -## Testing - -A test app (`test_multiple_dirs.R`) has been created to verify the fix works correctly. The test app includes three `shinyDirChoose` buttons to ensure that: - -1. Each modal opens and closes correctly -2. The "Select" button properly closes the modal -3. Multiple modals don't interfere with each other -4. Keyboard shortcuts (Enter, Escape) work correctly - -## Files Modified - -- `inst/www/shinyFiles.js`: Fixed modal creation and event handlers -- `test_multiple_dirs.R`: Test app to verify the fix -- `FIX_MULTIPLE_MODALS.md`: This documentation - -## Impact - -This fix resolves the issue where modals don't close after clicking "Select" when using multiple `shinyDirChoose` buttons. The fix is backward compatible and doesn't affect single modal usage. diff --git a/test_multiple_dirs.R b/test_multiple_dirs.R deleted file mode 100644 index bdc7092..0000000 --- a/test_multiple_dirs.R +++ /dev/null @@ -1,63 +0,0 @@ -library(shiny) -library(shinyFiles) -library(fs) - -ui <- fluidPage( - titlePanel("Test Multiple Directory Choosers"), - sidebarLayout( - sidebarPanel( - h4("Multiple Directory Choosers"), - p("This test app has multiple directory choosers to verify that modals close correctly."), - hr(), - shinyDirButton("dir1", "Directory 1", "Please select first directory"), - br(), br(), - shinyDirButton("dir2", "Directory 2", "Please select second directory"), - br(), br(), - shinyDirButton("dir3", "Directory 3", "Please select third directory"), - hr(), - p("Try opening multiple directory choosers and clicking 'Select' - the modal should close properly.") - ), - mainPanel( - h4("Selected Directories:"), - verbatimTextOutput("dir1_output"), - verbatimTextOutput("dir2_output"), - verbatimTextOutput("dir3_output") - ) - ) -) - -server <- function(input, output, session) { - volumes <- c(Home = fs::path_home(), "R Installation" = R.home(), getVolumes()()) - - # Set up multiple directory choosers - shinyDirChoose(input, "dir1", roots = volumes, session = session) - shinyDirChoose(input, "dir2", roots = volumes, session = session) - shinyDirChoose(input, "dir3", roots = volumes, session = session) - - # Output the selected directories - output$dir1_output <- renderPrint({ - if (is.integer(input$dir1)) { - cat("Directory 1: No directory selected") - } else { - cat("Directory 1:", parseDirPath(volumes, input$dir1)) - } - }) - - output$dir2_output <- renderPrint({ - if (is.integer(input$dir2)) { - cat("Directory 2: No directory selected") - } else { - cat("Directory 2:", parseDirPath(volumes, input$dir2)) - } - }) - - output$dir3_output <- renderPrint({ - if (is.integer(input$dir3)) { - cat("Directory 3: No directory selected") - } else { - cat("Directory 3:", parseDirPath(volumes, input$dir3)) - } - }) -} - -shinyApp(ui = ui, server = server) From 3428b588b2900a2146eb6195b2a73297f803526e Mon Sep 17 00:00:00 2001 From: Kamil Foltynski Date: Fri, 8 Aug 2025 15:35:04 +0200 Subject: [PATCH 3/3] Add Kamil Foltynski as contributor (ctb) Added Kamil Foltynski to the Authors@R field in DESCRIPTION with role 'ctb' (contributor) to recognize the contribution to fixing the multiple modal issue in shinyDirChoose functionality. --- DESCRIPTION | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index ae26768..70d2551 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -16,7 +16,10 @@ Authors@R: role = "aut"), person(given = "Eric", family = "Nantz", - role = "aut")) + role = "aut"), + person(given = "Kamil", + family = "Foltynski", + role = "ctb")) Maintainer: Thomas Lin Pedersen Description: Provides functionality for client-side navigation of the server side file system in shiny apps. In case the app is running