Skip to content
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

Improved Excel button #122

Merged
merged 23 commits into from
Nov 11, 2024
Merged

Improved Excel button #122

merged 23 commits into from
Nov 11, 2024

Conversation

aclark02-arcus
Copy link
Collaborator

@aclark02-arcus aclark02-arcus commented Oct 29, 2024

Closes #96. This PR is currently an initial placeholder that generates the most basic of excel buttons using the "Buttons" extension for {DT}.

Some possible enhancements include:

  • styling and/or repositioning the button
    • Moved button to right-hand side
  • making the button disappear or disabled when no data exists in the table
    • Button only shows up on common forms & study forms, plus the queries table
  • Add excel button option to golem config
  • styling the output in excel to maintain bold (new / updated) records
  • Improving the the specificity of the filename (Right now, all tables output a generic name).

@aclark02-arcus aclark02-arcus marked this pull request as ready for review October 31, 2024 14:34
@aclark02-arcus aclark02-arcus marked this pull request as draft October 31, 2024 14:35
@LDSamson
Copy link
Collaborator

LDSamson commented Nov 1, 2024

It would be nice if we could reposition it is not the primary visual that people should look at when being presented by the table. Users will look for it if needed. Should we also disable it by default and only enable in the tables in which it is needed? Or add a toggle in the app's settings enable download excel buttons or something similar?

@aclark02-arcus aclark02-arcus changed the base branch from dev to devex November 4, 2024 02:39
@aclark02-arcus
Copy link
Collaborator Author

aclark02-arcus commented Nov 4, 2024

View of filename's when downloading listings from different forms is below. Naming convention is as follows:

{"clinsight", [form], [patient], [timestamp]}

Notice that if "show all patients" is selected, the filename will reflect this.

image

@aclark02-arcus
Copy link
Collaborator Author

aclark02-arcus commented Nov 4, 2024

Moved button to right-hand side so that it's not the first thing you see, and converted the text to just an icon

image

@aclark02-arcus aclark02-arcus marked this pull request as ready for review November 4, 2024 17:09
@aclark02-arcus
Copy link
Collaborator Author

@jthompson-arcus, got the coveted ✅

@aclark02-arcus
Copy link
Collaborator Author

eh, converting to draft since I lost the ✅ when merging dev -> devex -> ac-96. Will need to re-assess R CMD Check

@aclark02-arcus aclark02-arcus marked this pull request as draft November 6, 2024 03:29
@aclark02-arcus aclark02-arcus changed the title Add basic excel button Improved Excel button Nov 6, 2024
@aclark02-arcus aclark02-arcus marked this pull request as ready for review November 6, 2024 16:04
@aclark02-arcus
Copy link
Collaborator Author

@jthompson-arcus, this is ready for review.

Regarding one of your earlier questions, I chose the path of creating a helper function to configure DT ext / options instead of altering custom_datatable() because this download button only impacts a few tables and I didn't want to add another argument for input$show_all_data etc.

But, it's all up for debate, so I'm all ears if you have comments.

Copy link
Collaborator

@jthompson-arcus jthompson-arcus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments below.

DESCRIPTION Show resolved Hide resolved
R/mod_queries.R Show resolved Hide resolved
inst/golem-config.yml Show resolved Hide resolved
@aclark02-arcus aclark02-arcus merged commit bd9d8e0 into devex Nov 11, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants