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

#2217 - Add UI limit for chat message #2272

Merged
merged 5 commits into from
Aug 6, 2024

Conversation

SebinSong
Copy link
Collaborator

closes #2217

Currently CHATROOM_MAX_MESSAGE_LEN = 20000 and I've added maxlength attribute for it (which makes sure string beyond that limit gets cut off), but not that n/20000 indicator UI because I thought it will be rare that the user types messages longer than that and having it won't be that useful compared to other places.
But let me know if we need it in place here too.

@SebinSong SebinSong self-assigned this Jul 30, 2024
@SebinSong
Copy link
Collaborator Author

stringMax(CHATROOM_MAX_MESSAGE_LEN) validator in chatroom.js contract has already been added before. So no need to take care here.

Copy link

cypress bot commented Jul 30, 2024



Test summary

112 0 10 0


Run details

Project group-income
Status Passed
Commit 794b731 ℹ️
Started Aug 5, 2024 10:44 PM
Ended Aug 5, 2024 10:55 PM
Duration 10:57 💡
OS Linux Ubuntu - 20.04
Browser Electron 89

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Nice work @SebinSong!

One issue I found: the UI gets kind of broken in Firefox when I past a very large file. I couldn't even check if the text is being cut off because I'm not able to scroll the page or the input box when it's like this:

Screenshot 2024-08-02 at 1 46 37 PM

Can you maybe add a height limit to the input box itself, and then add a scrollbar that appears so that the contents can be scrolled? (Please verify it also works well on mobile too when a very large amount of text is copy/pasted)

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Nice work @SebinSong, I think this is a good height for mobile:

Screenshot 2024-08-03 at 11 32 14 AM

For desktop, I would make it slightly taller.

However, the most important issue is that still I cannot scroll the input box. Can you? When I try to scroll using the scroll wheel it doesn't work. I would also maybe make a vertical scrollbar visible on the righthand side when the text overflows past the max height.

@SebinSong
Copy link
Collaborator Author

@taoeffect
Increased max-height for desktop size screen to 12.25rem and also fixed the hidden scrollbar bug.

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Excellent!

@taoeffect taoeffect merged commit 2f66832 into master Aug 6, 2024
2 checks passed
@taoeffect taoeffect deleted the sebin/task/#2217-message-max-length branch August 6, 2024 01:20
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.

Add UI limit to chatroom max message length
2 participants