Add table text wrap toggle#252
Conversation
(cherry picked from commit 2d34a1d3eca92d059bb2ddd8bc6b53a2b41e465b)
jthompson-arcus
left a comment
jthompson-arcus
left a comment
There was a problem hiding this comment.
Enabling text wrap does not appear to be working. For instance checkout subject BEL_04_772 and the Medication tab. There are 15 records. If you "enable text wrap" you can only view 10 of them.
Co-authored-by: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Co-authored-by: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
|
I think generally I'm a bit confused by this PR. In the common events the text wrapping is disabled on form level review. I assume this is to mitigate issues from having all records visible but the similar checking doesn't seem to be in place for the toggle to see all subjects. The toggle doesn't disappear at the form level either so it all feels a bit messy to me. |
Hi, thanks for the review. Initially, the idea was to discourage to use it in form level review, but not to disallow it. Do you think it should be completely forbidden? |
If this feature is something desired, I personally would think about turning pagination back on if it is triggered. You are re-rendering the table anyways. |
@jthompson-arcus Text wrap works it's just that not all entries are shown indeed which I did not expect.. do you know why this is the case? I will try to switch to pagination again and see if that works. |
# Conflicts: # tests/testthat/_snaps/app_feature_01/app-feature-1-003_.png # tests/testthat/_snaps/app_feature_03/app-feature-3-001.json # tests/testthat/_snaps/app_feature_03/app-feature-3-002.json
# Conflicts: # DESCRIPTION # NEWS.md # R/mod_review_form_tbl.R # R/mod_study_forms.R # inst/golem-config.yml # tests/testthat/test-app_feature_03.R
jthompson-arcus
left a comment
jthompson-arcus
left a comment
There was a problem hiding this comment.
I still do not understand why we would not want the end user to be able to enable wrapping when reviewing by form. The word wrap causes the table to display paginated, so we should allow that view for form level reviews as well.
Also, perhaps padding between the AE tables should be considered.
|
Text wrap is now consistent between form-level review and just pressing the It switches to no text wrapping when clicking on 'show all participants'. This is because in this mode, it is probably easier to compare lots of data if the data is on one row. But you can still switch on text wrap if desired. I also added extra padding under the SAE table as suggested and removed the tooltip warning since it seems like text wrap is still okay for large datasets when pagination is enabled (which is the case here) |
jthompson-arcus
left a comment
jthompson-arcus
left a comment
There was a problem hiding this comment.
I know this is turning into the PR of a thousand change requests but...I found a bug.
If you select rows to review and then toggle "enable text wrapping", the rows will still be cached and you can hit the "save" button but the rows in the table do not show being selected.
# Conflicts: # DESCRIPTION # NEWS.md # inst/golem-config.yml # tests/testthat/test-app_feature_03.R
@jthompson-arcus thanks for spotting this. Now resolved in this commit: 2c0a2e1. |
| # Any time the data in the form table is updated, "show all" is toggled, | ||
| # or the subject being viewed is changed, the server data for the datatable | ||
| # needs to be updated | ||
| # Triggers when server data needs to be updated. Also triggers for each | ||
| # change in pending review records (e.g. a checkbox in column `Reviewed` | ||
| # is toggled on or off). |
There was a problem hiding this comment.
@jthompson-arcus changed the description here and in lines 173-174 a bit to capture the 'why' because it always take me some time to familiarize with the logic here after a while. If you don't like it or have suggestions for better description please let me know.
jthompson-arcus
left a comment
jthompson-arcus
left a comment
There was a problem hiding this comment.
Looks good to me now. Thanks for the updates.
2 participants
Mostly solves #224 .
Provides the use to enable text wrapping. The option is off by default, and a warning is shown that it might be slow to enable it for big tables