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

Add filter for database overview #619

Merged
merged 17 commits into from
Oct 17, 2023
Merged

Add filter for database overview #619

merged 17 commits into from
Oct 17, 2023

Conversation

Eduardodudu
Copy link
Collaborator

Task

Feature task on the option to upload all packages #586

This PR adds

  1. Adds a filter on top of the table for database overview

image

@AARON-CLARK AARON-CLARK marked this pull request as ready for review October 10, 2023 11:07
@AARON-CLARK
Copy link
Collaborator

Sorry @Eduardodudu, I saw this PR marked as "Draft" so I thought it was still under development. I see now that you requested my review several months ago. I pushed a couple commits to update your branch. We'll see how the CI checks fair.

@AARON-CLARK
Copy link
Collaborator

AARON-CLARK commented Oct 10, 2023

@Eduardodudu, I also pushed a few commits to change the aesthetics of the filters. Previously, they were pretty large, using bootstrap styles. I converted to a more "plain" / minimalist look that takes up less visual real estate:

image

In addition, I'm wondering if you could explore one more thing. Right now, we've been converting everything to character so that we can manipulate how blank cells look. AKA, we've replaced NAs and empty chars ("") with "-". As a result, this holds us back from leveraging the cool filtering widgets that come by default with DT's filter, as seen below. There are range sliders for dates and numeric values, which would come in handy. Also, factor variables get a handy dropdown menu of values to choose from, which would be useful for decision and decision_by columns. I've pushed a commit to restore these data types.However, it looks like it still doesn't work unless we remove the formattable::as.datatable(formattable::formattable()) code, which adds a lot value to our score, decision, and explore metrics columns.

image

Wondering if we can somehow maintain the {formattable} code AND still leverage the handy filtering widgets?

@AARON-CLARK
Copy link
Collaborator

Hey @Jeff-Thompson12, I'm getting pulled to another project for the next few days. Wondering if you can help resolve these broken tests before the new release?

@Jeff-Thompson12
Copy link
Collaborator

Yeah, I got you bro

@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #619 (ba5e4d9) into dev (964faed) will increase coverage by 0.04%.
Report is 3 commits behind head on dev.
The diff coverage is 100.00%.

❗ Current head ba5e4d9 differs from pull request most recent head 1de5f6f. Consider uploading reports for the commit 1de5f6f to get more accurate results

@@            Coverage Diff             @@
##              dev     #619      +/-   ##
==========================================
+ Coverage   72.88%   72.93%   +0.04%     
==========================================
  Files          33       33              
  Lines        4762     4770       +8     
==========================================
+ Hits         3471     3479       +8     
  Misses       1291     1291              
Files Coverage Δ
R/mod_databaseView.R 91.47% <100.00%> (+0.40%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@AARON-CLARK
Copy link
Collaborator

Hi @Robert-Krajcik, since Jeff and I both pushed commits to this branch, wondering if you could provide the final review?

@Robert-Krajcik
Copy link
Contributor

Hi @AARON-CLARK Sure.

@Robert-Krajcik
Copy link
Contributor

Hi @AARON-CLARK and @Jeff-Thompson12
The basic filtering works. For me, everything to the right of score is greyed out, probably because I haven't populated it.
Not sure filtering on explore metrics makes sense, but I assume it's always greyed out.

Just wondering if we could do something with the score, like allowing a range of values, or even "<" or ">"?
image

@AARON-CLARK
Copy link
Collaborator

AARON-CLARK commented Oct 17, 2023

... everything to the right of score is greyed out, probably because I haven't populated it.

Whenever a column is populated with the a constant value, DT is smart enough to know that there is no variability and filtering is pointless.

Not sure filtering on explore metrics makes sense, but I assume it's always greyed out.

It is, I added a little code to make sure the last column's filtering widget was disabled.

Just wondering if we could do something with the score, like allowing a range of values, or even "<" or ">"?

Right now, DT does offer a range slider for numeric variables, but it's not showing up because we're using formattable::as.datatable() instead of DT::datatable(). Creating a workaround is going to make a little too much extra work, so we are going to leave things as-is for this PR.

Thanks @Robert-Krajcik! If everything else looks okay, feel free to merge.

Copy link
Contributor

@Robert-Krajcik Robert-Krajcik left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Robert-Krajcik Robert-Krajcik merged commit e19427b into dev Oct 17, 2023
3 checks passed
@Eduardodudu
Copy link
Collaborator Author

Hey @AARON-CLARK!

Sorry for the late response, I was on vacation

I see that the task is already closed.

Thanks for the return @AARON-CLARK !

@AARON-CLARK AARON-CLARK deleted the ea-586 branch October 20, 2023 00:34
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.

4 participants