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

Updating hex text input to work with or without a hashtag #1

Closed
wants to merge 1 commit into from

Conversation

DaveZMB
Copy link

@DaveZMB DaveZMB commented Jan 27, 2022

UAT feedback "Missing # in color code will not work"

The color picker is built using a 3rd party plugin, unfortunately that plugin does not support hex values that don't include a starting # so I had to update their code to make it work.

There are multiple places in the code that ensure the hex value contains a starting #, so rather than trying to update them all to work with & without a # I am simply ensuring that all hex values start with a # before doing any processing.

I have submitted a bug report and a pull request to the original package, but it doesn't look like there have been any changes since the master branch was created so I'm not very hopeful that they will pull it in.

So I'm thinking to switch mcms to use our [fixed] forked version, but could be easily convinced that we should just change to a different 3rd party color picker instead.

Fwiw, this plugin was selected by Kristine because it looked and behaved how she wanted. I'm sure we can find another plugin that is pretty similar and doesn't have this # issue, but there would probably be a bit more testing involved in switching plugins.

@groovetrain
Copy link

If this works, it's my opinion that we should move forward with our own forked version, since it works. It's less work, and whatever problems there are, we already know about them.

Comment on lines +356 to +359
if (typeof el.value !== 'object' && el.value.indexOf('#') !== 0) {
el.value = '#' + el.value;
}

Choose a reason for hiding this comment

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

I like specifically checking for there being a '#' as the first character, I'd amend the replacement as follows, in case it's later on in the string (perhaps doing too much sanitation, but whatever, I'll leave it up to you if you think it's important enough)

el.value = '#' + el.value.replace(/[^0-9A-F]/ig, '').substr(0,6);

Choose a reason for hiding this comment

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

I also know you probably spent an annoying amount of time finding where to actually make this change, so for that I applaud you! :)

@DaveZMB DaveZMB closed this Jan 28, 2022
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.

2 participants