-
Notifications
You must be signed in to change notification settings - Fork 1
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
Improve user interaction & error catching for prompts #80
Conversation
I realise this a very big PR - sorry about that! The most helpful thing will be thorough user testing. If you look through the code changes that's a bonus. User testing:
|
User testing notes (pt 1):
|
(REPLY) User testing notes (pt 1):✅ [1] allowing for lower case (y/n) not just upper case (Y/N) as user input
✅ [2] instead of having users specify 'N' every time, it may be simpler to press enter if no note or enter any string to write a note
✅ [3] if you repeat row number it'll ask you to repeat the classification of that row; could make the set of rows distinct so (2,2,2,3) -> (2,3)
✅ [4] 'Press enter to accept the auto categorisations for table CHILD or enter each row you'd like to edit:' allows you to edit categorisations outside the auto categorisations, may be helpful to be explicit here that you can edit any row and leave code as is, or, change code to only accept the rows that have been auto categorised if you want it specific
✅ ❓ [5] Although doesn't allow invalid datatype, it allows entering of invalid row numbers - could prompt user this is invalid if not < length(variable) & type(int) or something similar, the same way you've done for data type. Can also just leave it as it repeats prompt to enter the row numbers anyway.
✅ ❓ [6a] Enter each table number you want to process in this interactive session:' - can this be changed to specify the delimiter the user should use (for eg: 'Enter each table number you want to process, separated by commas, in this interactive session') and input all in one line? may be clearer
✅ ❓ [6b] also not clear how to exit, so specifying some sort of set (1,3,5) and exiting may be nicer but not necessary :)
✅ [7] 'Categorise this data element into one or more domains, e.g. 5 or 5,8:' - input other than integer (and comma delimiters) are allowed here, though if it doesn't break anything its not really a problem
✅ ❓ [8] I remember there being a feature to select which variables you want to categorise for each table - is this feature removed? it doesn't show up when I process the tables
|
@Rainiefantasy see my replies above. I have merged this PR but please look through my replies to see if we need to change anything else. Thanks for your feedback! |
Closes #74
Closes #62
Closes #71
readline
function with thescan
function.tryCatch
to avoid the code crashing if the user gives the wrong input for browsing the metadata tablesCloses #75
Though I like the idea in this Issue in terms of user flexibility, I actually think it is simpler to leave this for now and return for a later enhancement.
Code in this PR has been changed so that: