Skip to content

Conversation

@LDSamson
Copy link
Collaborator

@LDSamson LDSamson commented Dec 9, 2025

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

grafik

@LDSamson LDSamson marked this pull request as ready for review December 12, 2025 16:12
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.

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.

LDSamson and others added 2 commits December 17, 2025 17:13
Co-authored-by: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
Co-authored-by: Jeff Thompson <160783290+jthompson-arcus@users.noreply.github.com>
@jthompson-arcus
Copy link
Collaborator

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.

@LDSamson
Copy link
Collaborator Author

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?

@jthompson-arcus
Copy link
Collaborator

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.

@LDSamson
Copy link
Collaborator Author

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.

@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
@LDSamson LDSamson marked this pull request as draft December 23, 2025 11:03
@LDSamson LDSamson removed the request for review from jthompson-arcus December 23, 2025 11:03
@LDSamson LDSamson assigned LDSamson and unassigned jthompson-arcus Dec 23, 2025
# 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
@LDSamson
Copy link
Collaborator Author

LDSamson commented Jan 5, 2026

The toggle doesn't disappear at the form level either so it all feels a bit messy to me.

I am a bit confused by this comment. The toggle should disappear when using form_level review. Is this not the case for you?
I updated the PR, pagination is now again enabled when text_wrap is enabled. Let me know what you think!

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.

@LDSamson LDSamson marked this pull request as ready for review January 5, 2026 17:08
@LDSamson LDSamson assigned jthompson-arcus and unassigned LDSamson Jan 5, 2026
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