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

Manually adding hex color in color picker dialog doesn't pre-fill with hashtag #565

Closed
kd8ssq opened this issue Jun 26, 2017 · 9 comments
Closed
Assignees
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. type:feature A feature request.

Comments

@kd8ssq
Copy link

kd8ssq commented Jun 26, 2017

Are you reporting a feature or a bug?

Task

Check if the issue is already reported

None Found

Provide detailed reproduction steps (if any)

  1. In CKeditor, highlight some text
  2. In the toolbar, click "Text Color" and then "More Colors..."
  3. In the text field above the "Clear" button, enter in any hex color without the preceding hashtag (ex. 333333)
  4. Click Ok

Expected result

Since hex colors need to have the preceding hashtag to render properly, if a hashtag isn't entered, I would assume it would automatically be added. The editor seems to display the color without the hashtag (sometimes you have to switch in and out of Source mode to get it to show up, but it does), but as soon as you go to view the text on a web page, the color doesn't display properly because the browser cannot render without the hashtag.

Actual result

No hashtag is added and the browser cannot render the style

Other details

Tested in Ckeditor Demo with same results

  • Browser: Chrome 58.0.3029.110
  • OS: Windows 10
  • CKEditor version: 4.5.7
  • Installed CKEditor plugins: Full package w/ codemirror,codeembed,iframe,saveeditor,canceleditor,sourcedialog,uploadimage

color picker dialog

@mlewand
Copy link
Contributor

mlewand commented Jun 27, 2017

It's not a bug, but sounds like a nice and simple feature request.

The reason why it does not add hash aggresively is that this field accepts any valid CSS value, so you could type red, #f00, #ff0000, rgb(255,0,0) etc.

I'd say that it could adding a missing hash only if the value matches a regexp allowing only chars 0-9a-f and with exactly 3 or 6 chars. In this case after change it could automatically add a missing # char.

@mlewand mlewand added good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. type:feature A feature request. labels Jun 27, 2017
@mlewand mlewand added this to the Backlog milestone Jun 27, 2017
@msamsel msamsel self-assigned this Jun 27, 2017
@f1ames
Copy link
Contributor

f1ames commented Jul 5, 2017

The fix for colordialog landed on major (#580). However, due to the fact that colorbutton does not utilize colordialog API for getting selected color this issue is still not fixed. It will be fixed by #590 (which is about switching to colordialog API in colorbutton).

@msamsel
Copy link
Contributor

msamsel commented Jul 24, 2017

#590 is marged, so we can close this issue as a done.

@msamsel msamsel closed this as completed Jul 24, 2017
@mlewand
Copy link
Contributor

mlewand commented Aug 16, 2017

@f1ames @msamsel I'm a bit lost here, it's my understanding that this issue is a duplicate of #607 - right? If so please, update it with a proper labels and remove from backlog.

I'm removing it from a backlog anyway, please add a proper state label.

@mlewand mlewand removed this from the Backlog milestone Aug 16, 2017
@f1ames
Copy link
Contributor

f1ames commented Aug 17, 2017

@mlewand :
#580 was about adding # to hex color representation when colordialog is closed so every plugin utilising its API will benefit from it.
#590 was about utilizing this colordialog API in colorbutton as it was not used there.

This two fixes were needed to resolve this issue.

#607 was about changing color/background-color of table cells using color hex representaion without #. As it already utilizes colordialog API, it benefited from #580. However, this (#565) and #607 are two different issues as one concerns colorbutton and the other one table cell properties dialog.

I think it should be added to 4.7.2 4.8.0 milestone?

@brandonfarber
Copy link

This issue seems to be present in 4.11.4. I can't find an online build of CKEditor 5 that allows you to choose the font color (i.e. using the color picker) to verify if the issue exists in CKEditor 5 or not.

@f1ames
Copy link
Contributor

f1ames commented Apr 12, 2019

@brandonfarber Can you post reproduction steps? It looks for me that it works fine with 4.11.4.

CKEditor 5 doesn't have online builder at the moment. It also doesn't provide color dialog as in CKEditor 4 so this issue will not be present there.

@brandonfarber
Copy link

  1. Go here: https://ckeditor.com/docs/ckeditor4/latest/examples/fullpreset.html
  2. Select "Apollo 11" in the editor
  3. Click the color button
  4. Click the "More Colors" link at the bottom
  5. In the color field type FF0000 (no hash tag) and click out of the field. You get this:
    image
  6. If you type #FF0000 instead you get this (note the "Selected Color" field):
    image

Upon further testing I do see if you click the "Ok" button the manually typed color code does work and colorizes the selected text, so I guess it's only the "Selected color" field that does not update when the manually supplied color field is blurred when you don't supply a hash tag. Using latest stable Chrome on Windows 10.

@f1ames
Copy link
Contributor

f1ames commented Apr 15, 2019

Upon further testing I do see if you click the "Ok" button the manually typed color code does work and colorizes the selected text, so I guess it's only the "Selected color" field that does not update when the manually supplied color field is blurred when you don't supply a hash tag.

Yes, that's exactly what happens and this issue was to fix the behavior after pressing OK. If you find the behavior with adding hash to manually inserted color on blur, feel free to open a new feature request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. type:feature A feature request.
Projects
None yet
Development

No branches or pull requests

5 participants