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

added warning when changeset comment length > 255 chars #9392

Closed

Conversation

alanb43
Copy link
Contributor

@alanb43 alanb43 commented Nov 27, 2022

Added a simple fix for the issue described in #9374 and #7943. I simply insert a warning message / box in the same format that the existing Google warning appears when the user exceeds 255 characters in their changeset message.

#6817 describes some edge cases that seem to work fine with this change, shown below:

"ZALGO" contains 58 javascript characters (source), here we see exactly 255 characters being calculated as 'good' using 4 * 58 of the demon string + 18 normal chars.

image

If we add one, we see the new warning:
image

I also have come to understand that there is a more in-depth fix being worked on in #9390, but this was made as a much simpler fix that follows suit of an existing warning.

@tyrasd
Copy link
Member

tyrasd commented Nov 27, 2022

Thanks for this, @alanb43.

The main difference to #9390 is that this PR only addresses the issue for the changeset comment field. The problem of too long strings is however also present on input fields for any OSM tag (see bug #6817). It's not as often encountered for regular tags, but there are also some somewhat common cases (e.g. a long note, description or source tag).

I think that your PR has the benefit that it stylistically matches the existing warnings for other changeset comment mistakes. I think it would be best for the changeset comment if we combine the length indicator widget from #9390 with the warning message from this PR. What do you think?

@tyrasd tyrasd added field An issue with a field in the user interface enhancement An enhancement to make an existing feature even better labels Nov 27, 2022
@alanb43
Copy link
Contributor Author

alanb43 commented Nov 27, 2022

I agree with this and have been tinkering for a bit now trying to do it, without much luck. Will let you know if I make progress here though!

@tyrasd
Copy link
Member

tyrasd commented Nov 28, 2022

This is now integrated into #9390. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement to make an existing feature even better field An issue with a field in the user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants