junichi11 / netbeans-color-codes-preview

Show colors in an editor's sidebar
https://netbeans-vm.apache.org/pluginportal/catalogue/?id=24
Apache License 2.0
32 stars 8 forks source link

CSS HSLA color value causes Unexpected Exception in NB12 #47

Closed DelPoint closed 3 years ago

DelPoint commented 3 years ago

Describe the bug Valid HSLA color value for CSS background/color property in a CSS file causes Unexpected Exception in Netbeans 12

Steps to Reproduce

  1. Open a new/existing CSS file in a PHP project.
  2. Add/select a rule.
  3. Set color or background property's value to : hsla(60, 100%, 50%, 0.5)
  4. See error in NB Notification windows: Unexpected Exceptions.
  5. Check the margin of the window: color not displayed/shown.

(I can send NB's log/error report if you wish, but it is really easy to reproduce. the problem.)

Expected behavior Color preview displayed as usual.

Screenshots

Environments (please complete the following information):

Additional context

junichi11 commented 3 years ago

Reproducible. Thanks for reporting it.

DelPoint commented 3 years ago

You are welcome :+1:

junichi11 commented 3 years ago

@DelPoint I've fixed it. Could you verify it? https://github.com/junichi11/netbeans-color-codes-preview/releases/tag/v0.13.2 (0.13.2.1-dev) If there is no problem, I'll upload it to PP as the new version. nb-color-codes-preview-issue47

DelPoint commented 3 years ago

Hi,

The actual issue

I've checked it, and the Unexpected Exception is definitely gone.

Transparency vs alpha

However, your Transparency slider/value is still very counter intuitive from a user's view point:

plg_color-codes-preview_transparency

First of all, I would rename Transparency to Alpha. That's the term/name used in CSS spec: https://drafts.csswg.org/css-color/#the-hsl-notation - and Netbeans is for devs eventually. I would note however, that e.g. on Mozilla dev. site they are also 'confusing' the user a bit using the words transparency/opacity interchangeable: https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/hsla() Transparency vs. opacity is a bit confusing to understand, especially for non-English speakers.

Then I would use for alpha the range of 0...1, instead of 0...100. Currently, you're using something like this in the background to calculate the output of the alpha value from the user input in your current Transparency slider/input field: alpha = 1 - transparency/100 That is, if I want zero alpha, I have to set 100 transparency, and vice-versa. Think about the user! He want to use a 'target alpha' value, but he has to calculate 'backwards', and he need to set

That's not very user friendly :D

Resolution/accuracy of the values

INMHO you should enable at least 1 or 2 figure accuracy (1 or 2 digits after decimal sign) for the Hue/Saturation/Lightness values in the code. We may argue if it is necessary, or too hard to handle a value of 60.25% for saturation/lightness, or what is the point to have a such high 'resolution', but according to the CSS spec:

So, currently the plugin is unable to display the color of a hsla value like this: hsla(60.1, 55.23%, 65.456%, 0.789), while this is a valid definition. Obviously, to support these real numbers in hsla function would require the UI and the internal rounding procedure of your plugin to change. And I definitely would agree if you would say that this kind of color definition is so rarely used in real word that it is not worth the effort. So, this is up to you.

Sorry for the long reply.

junichi11 commented 3 years ago

I've checked it, and the Unexpected Exception is definitely gone.

OK, I'll close this later.

However, your Transparency slider/value is still very counter intuitive from a user's view point: ...

Don't write another issue in one issue.

DelPoint commented 3 years ago

Don't write another issue in one issue.

I didn't mean it as an "issue". Technically it is not. I mean: it is not a bug that prevents the plugin to work. Your fix works. All other things were mentioned only because of my personal opinion. Although, you may argue it is an "usability issue". If you wish I can open a new issue about those transparency/opacity/slider/decimal digits thingies :-)

junichi11 commented 3 years ago

Well, this plugin uses the JColorChooser. It has all panels which your image file has (HSL, RGB, and so on) by default. (i.e. It means I don't create each panel.) So, I'm not going to do anything about it.

junichi11 commented 3 years ago

I'll improve it only a bit. See #48

DelPoint commented 3 years ago

Ok, understood. I had have a feeling that this is about some pre-built JAVA specific component/library.

48 : looks OK to me, if you ask me :)