fix(ui-contract-editor): Highlight read-only text clause template - i242

[K-Kumar-01]

Signed-off-by: k-kumar-01 kushalkumargupta4@gmail.com

Closes #242

Users are able to highlight read-only text in the clause template.

Changes

• user-select property changed in clause wrapper

Screenshots or Video

Author Checklist

• [x] Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
• [ ] Vital features and changes captured in unit and/or integration tests
• [x] Commits messages follow AP format
• [ ] Extend the documentation, if necessary
• [ ] Merging to master from fork:branchname
• [ ] Manual accessibility test performed
  • [ ] Keyboard-only access, including forms
  • [ ] Contrast at least WCAG Level A
  • [ ] Appropriate labels, alt text, and instructions

[jolanglinais]

I don't think this is the implementation we'd want. If you click anywhere in the clause while readOnly is true, the entire clause is selected.

[K-Kumar-01]

I don't think this is the implementation we'd want. If you click anywhere in the clause while readOnly is true, the entire clause is selected.

I had doubts whether we had to make it all selectable or some parts of it so I stuck to user-select:all I think user-select: none would work as it would make the content selectable which might be desired. @irmerk Would it be fine If some parts are highlightable instead if the entire clause in the readOnly state

[jolanglinais]

I think @Michael-Grover should take the lead on this one as far as UX goes.

[K-Kumar-01]

@Michael-Grover Any updates on this?

[dselman]

@Michael-Grover Any updates on this?

@Michael-Grover is on vacation this week. The behaviour I think users will want is that they can select any of the text (in whole, or in part) of the document, including clauses - irrespective of whether the editor is in read/write or read-only mode.

As far as I can tell this seems to be the case, in Safari at least. I tested that copy/paste worked into a Google Doc and got quite good results. Paste back into the editor had some issues, but that should be covered under a separate issue.

[K-Kumar-01]

@Michael-Grover is on vacation this week. The behaviour I think users will want is that they can select any of the text (in whole, or in part) of the document, including clauses - irrespective of whether the editor is in read/write or read-only mode.

@dselman Selecting any part of the text or whole can be implemented. I just had a doubt that if one can select any part of the text then they can edit the text in clause(when readOnly:false), if that is acceptable then I can implement this feature. There is no issue in readOnly knob regarding this.

[Michael-Grover]

Hi @K-Kumar-01 , thanks again for taking on this issue. I'm back from my vacation, so I can give some clarification about the expected behavior.

I think we should allow the user to choose what text they want to highlight, instead of forcing them to highlight the entire clause template. See the video for clarification.

https://www.loom.com/share/8c31e6837f8546b784f915bb26ede005

Here are some examples of a user choosing what they want to highlight in read-only text.

[K-Kumar-01]

Hi @K-Kumar-01 , thanks again for taking on this issue. I'm back from my vacation, so I can give some clarification about the expected behavior.

I think we should allow the user to choose what text they want to highlight, instead of forcing them to highlight the entire clause template. See the video for clarification.

https://www.loom.com/share/8c31e6837f8546b784f915bb26ede005

Here are some examples of a user choosing what they want to highlight in read-only text.

@Michael-Grover Thanks for the clarification. I had a small doubt. If readOnly: false then should the text inside the clause template be editable? Rest for readOnly: true I think I got what I need to do.

[K-Kumar-01]

Hi @K-Kumar-01 , thanks again for taking on this issue. I'm back from my vacation, so I can give some clarification about the expected behavior. I think we should allow the user to choose what text they want to highlight, instead of forcing them to highlight the entire clause template. See the video for clarification. https://www.loom.com/share/8c31e6837f8546b784f915bb26ede005 Here are some examples of a user choosing what they want to highlight in read-only text.

@Michael-Grover Thanks for the clarification. I had a small doubt. If readOnly: false then should the text inside the clause template be editable? Rest for readOnly: true I think I got what I need to do.

@Michael-Grover Any updates here?

[Michael-Grover]

@K-Kumar-01 The behavior of readOnly:false shouldn't change. This PR should only address allowing users to highlight text in a read only clause template.

[K-Kumar-01]

@Michael-Grover I have updated my PR to highlight the text while retaining the original functionalities. Let me know if there is any change required.

[dselman]

@K-Kumar-01 Thanks, the functionality looks good to me. I think we should get an engineer's review before merging, like @jeromesimeon or @irmerk

Selection seems strange (Safari) and it seems different if you select forwards, or select backwards.

Forwards:

Backwards:

[Michael-Grover]

@dselman that's not in read only mode, right?

I'm seeing the same behavior in Google Chrome.

It looks like in this PR, the user can no longer highlight a combination of clause template text and normal text in editing mode. Users can do this in the current version of storybook here https://ap-web-components.netlify.app/?path=/story/contract-editor--contract-editor. We want to keep this functionality from the current version of the storybook, so that users can highlight a combination of clause template text and normal text.

https://www.loom.com/share/9ff9ecd27fbf409ea244a1918b1a612c

[K-Kumar-01]

@K-Kumar-01 Thanks, the functionality looks good to me. I think we should get an engineer's review before merging, like @jeromesimeon or @irmerk

Selection seems strange (Safari) and it seems different if you select forwards, or select backwards.

Forwards:

Backwards:

@dselman @Michael-Grover I will try to find an alternative solution to fix this one

[K-Kumar-01]

@dselman @Michael-Grover I have updated my PR. Also a video showing the working of my PR https://www.loom.com/share/738f544d7cd442bb9075f3003ccc26b4 Let me know if there are any changes required.

[Michael-Grover]

Looks great, thanks @K-Kumar-01 !