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

K-Kumar-01 · 2021-02-15T10:04:52Z

Signed-off-by: k-kumar-01 [email protected]

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

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
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

Signed-off-by: k-kumar-01 <[email protected]>

jolanglinais · 2021-02-15T13:28:02Z

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 · 2021-02-15T20:34:26Z

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 · 2021-02-15T21:25:52Z

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

K-Kumar-01 · 2021-02-17T11:27:22Z

@Michael-Grover Any updates on this?

dselman · 2021-02-17T12:02:02Z

@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 · 2021-02-17T13:46:37Z

@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 · 2021-02-18T17:03:15Z

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 · 2021-02-18T19:46:22Z

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 · 2021-02-22T15:47:54Z

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 · 2021-02-22T16:34:54Z

@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.

Changed userSelect to text Main body contentEditable set false Variable wrapper contentEditable depend on readOnly prop Signed-off-by: k-kumar-01 <[email protected]>

K-Kumar-01 · 2021-02-22T18:09:15Z

@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.

Michael-Grover

@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

dselman · 2021-02-22T18:56:30Z

@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 · 2021-02-22T19:09:27Z

@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 · 2021-02-22T19:11:53Z

@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

Signed-off-by: k-kumar-01 <[email protected]>

K-Kumar-01 · 2021-02-23T06:06:03Z

@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.

dselman

Works great for me. Thank you!

Michael-Grover · 2021-02-23T15:11:02Z

Looks great, thanks @K-Kumar-01 !

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

b26ff85

Signed-off-by: k-kumar-01 <[email protected]>

jolanglinais assigned K-Kumar-01 Feb 15, 2021

jolanglinais added A11Y 🔓 Accessibility related Difficulty: Starter Type: Bug 🐛 Something isn't working Package: Contract Editor 💻 labels Feb 15, 2021

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

679972d

Changed userSelect to text Main body contentEditable set false Variable wrapper contentEditable depend on readOnly prop Signed-off-by: k-kumar-01 <[email protected]>

Michael-Grover requested review from Michael-Grover, dselman, jeromesimeon and jolanglinais February 22, 2021 18:16

Michael-Grover approved these changes Feb 22, 2021

View reviewed changes

K-Kumar-01 closed this Feb 22, 2021

K-Kumar-01 reopened this Feb 22, 2021

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

2dc84b5

Signed-off-by: k-kumar-01 <[email protected]>

dselman approved these changes Feb 23, 2021

View reviewed changes

dselman merged commit d6fe5d3 into accordproject:master Feb 23, 2021

K-Kumar-01 deleted the k-kumar-01/i242/clause-template-text branch February 23, 2021 18:55

mttrbrts mentioned this pull request Mar 10, 2021

chore(changelog): Add script to generate release notes #285

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

K-Kumar-01 commented Feb 15, 2021

jolanglinais commented Feb 15, 2021

K-Kumar-01 commented Feb 15, 2021

jolanglinais commented Feb 15, 2021

K-Kumar-01 commented Feb 17, 2021

dselman commented Feb 17, 2021

K-Kumar-01 commented Feb 17, 2021

Michael-Grover commented Feb 18, 2021

K-Kumar-01 commented Feb 18, 2021

K-Kumar-01 commented Feb 22, 2021

Michael-Grover commented Feb 22, 2021

K-Kumar-01 commented Feb 22, 2021

Michael-Grover left a comment

dselman commented Feb 22, 2021

Michael-Grover commented Feb 22, 2021

K-Kumar-01 commented Feb 22, 2021 •

edited

Loading

K-Kumar-01 commented Feb 23, 2021 •

edited

Loading

dselman left a comment

Michael-Grover commented Feb 23, 2021

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

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

Conversation

K-Kumar-01 commented Feb 15, 2021

Closes #242

Changes

Screenshots or Video

Author Checklist

jolanglinais commented Feb 15, 2021

K-Kumar-01 commented Feb 15, 2021

jolanglinais commented Feb 15, 2021

K-Kumar-01 commented Feb 17, 2021

dselman commented Feb 17, 2021

K-Kumar-01 commented Feb 17, 2021

Michael-Grover commented Feb 18, 2021

K-Kumar-01 commented Feb 18, 2021

K-Kumar-01 commented Feb 22, 2021

Michael-Grover commented Feb 22, 2021

K-Kumar-01 commented Feb 22, 2021

Michael-Grover left a comment

Choose a reason for hiding this comment

dselman commented Feb 22, 2021

Michael-Grover commented Feb 22, 2021

K-Kumar-01 commented Feb 22, 2021 • edited Loading

K-Kumar-01 commented Feb 23, 2021 • edited Loading

dselman left a comment

Choose a reason for hiding this comment

Michael-Grover commented Feb 23, 2021

K-Kumar-01 commented Feb 22, 2021 •

edited

Loading

K-Kumar-01 commented Feb 23, 2021 •

edited

Loading