-
Notifications
You must be signed in to change notification settings - Fork 2
Add table text wrap toggle #252
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
base: dev
Are you sure you want to change the base?
Conversation
(cherry picked from commit 2d34a1d3eca92d059bb2ddd8bc6b53a2b41e465b)
jthompson-arcus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
I am a bit confused by this comment. The toggle should disappear when using form_level review. Is this not the case for you? text wrap can indeed still be enabled when pressing show_all_participants but not when using form_level review, I thought for the latter it is probably not needed. |
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