Skip to content
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

Requests: Add Length Limit for Comment & Update Comment Editor UX #391

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Samk13
Copy link
Member

@Samk13 Samk13 commented Jul 3, 2024

❤️ Thank you for your contribution!

Description

Depends on: inveniosoftware/react-invenio-forms#244
Closes: inveniosoftware/invenio-app-rdm#2731

  • Set a maximum boundary for the comment message in the backend.
  • Make the max limit configurable by adding a config:

REQUESTS_COMMENT_CONTENT_MAX_LENGTH = 2500

Adapt the UI to this change by:

  • Adding live validation for comments
  • Disabling the submit comment button when the max length is exceeded or when it's 0.
  • Providing the config value for comment max length in the UI so it can change per instance config
    Adding a localized custom error message

Note: I set the maximum limit to a low value for demonstration purposes.
comment-length-4

When nearing the max limit, the error message might appear and then disappear when you hit space as the last character. This happens because the space character gets converted to   (which is 6 characters). However, when you type other letters, it turns back into a single space character.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Frontend

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.

@@ -142,7 +143,7 @@ def payload_schema():

Copy link
Member Author

@Samk13 Samk13 Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we set an upper limit to Log event as well?

Samk13 added a commit to Samk13/react-invenio-forms that referenced this pull request Jul 4, 2024
* This is useful when the input field has a character limit, allowing users to see the current character count.
* Related to inveniosoftware/invenio-requests#391
@Samk13 Samk13 force-pushed the set-comments-bounderies branch 7 times, most recently from 55d7fba to 8f93ee3 Compare July 5, 2024 22:10
@Samk13 Samk13 changed the title comments: set maximum length for comment content to 25000 characters Requests: Add Length Limit for Comment, Event Logging & Update Comments UI Jul 5, 2024
@Samk13 Samk13 changed the title Requests: Add Length Limit for Comment, Event Logging & Update Comments UI Requests: Add Length Limit for Comment & Update Comment Editor UX Jul 6, 2024
@Samk13 Samk13 marked this pull request as ready for review July 7, 2024 22:27
@Samk13 Samk13 requested review from ntarocco and kpsherva July 7, 2024 22:28
@Samk13 Samk13 force-pushed the set-comments-bounderies branch 5 times, most recently from e23919f to 6d48f8e Compare July 8, 2024 14:36
@Samk13 Samk13 added the bug Something isn't working label Jul 10, 2024
@Samk13 Samk13 force-pushed the set-comments-bounderies branch from 6d48f8e to ff13818 Compare July 10, 2024 09:48
* UI: Add live character count validation to TimelineCommentEditor
* UI: Connect config value through Redux store to make it available as we make config value configurable, this allowe the max limit to change dynamically as well
* These changes depending on thins PR: inveniosoftware/react-invenio-forms#244
* Disable submit button when comment exceed char count
* Disable submit button when it's < 1 char
* Show understandable error msg when exceeding limit
@Samk13 Samk13 force-pushed the set-comments-bounderies branch from ff13818 to f651cbc Compare August 13, 2024 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Requests: No Length Upper Boundary for Comment Messages
1 participant