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

Feedback about new Thing action pop-up #2847

Closed
lolodomo opened this issue Oct 30, 2024 · 20 comments
Closed

Feedback about new Thing action pop-up #2847

lolodomo opened this issue Oct 30, 2024 · 20 comments
Labels
bug Something isn't working main ui Main UI

Comments

@lolodomo
Copy link
Contributor

lolodomo commented Oct 30, 2024

In the pop-up, for a parameter that have QuantityType<Temperature> parameter as label and Descr QuantityType<Temperature> parameter as description, the label is properly displayed but the description is truncated to Descr QuantityType parameter.

image

For datetime and time, I see that we can enter only hours and minutes.

As a reminder, we decided to allow hours, minutes and seconds.

For type java.util.Date, it even leads to an error:

Input parameter 'dateParam': converting value '2024-10-31T08:07' into type java.util.Date failed! Input parameter is ignored

I noticed that for parameters of type INTEGER, it is allowed to enter a decimal value.
No way to block the filling of the decimal character ?

@lolodomo lolodomo added bug Something isn't working main ui Main UI labels Oct 30, 2024
@florian-h05
Copy link
Contributor

In the pop-up, for a parameter that have QuantityType parameter as label and Descr QuantityType parameter as description, the label is properly displayed but the description is truncated to Descr QuantityType parameter.

Just checked the code, this is because description allows to use HTML tags, <Temperarure> is rendered as HTML tag in that case. I fear we cannot fix this without breaking config dialogs for everything else based on config descriptions.

For datetime and time, I see that we can enter only hours and minutes.
As a reminder, we decided to allow hours, minutes and seconds.

Do you really think it is neccassary to allow entering seconds?
If yes, core needs to set step(size) to 1 to make the UI render seconds as well and I need to push a small fix to the UI to apply the step size.

I noticed that for parameters of type INTEGER, it is allowed to enter a decimal value.
No way to block the filling of the decimal character ?

I though that would be happening ... I will check what's going wrong here.

@lolodomo
Copy link
Contributor Author

Just checked the code, this is because description allows to use HTML tags, <Temperarure> is rendered as HTML tag in that case. I fear we cannot fix this without breaking config dialogs for everything else based on config descriptions.

Then we can keep as it is I believe.

Do you really think it is neccassary to allow entering seconds?

Yes, I am sure certain actions will require such precision.

If yes, core needs to set step(size) to 1 to make the UI render seconds as well and I need to push a small fix to the UI to apply the step size.

Let's go ;)

I though that would be happening ... I will check what's going wrong here.

Ok, thank you.

@florian-h05
Copy link
Contributor

I noticed that for parameters of type INTEGER, it is allowed to enter a decimal value.

Can you please share the config description returned by the API in that case?

@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 30, 2024

You can find it in our original PR.

But I can check again this evening.

@florian-h05
Copy link
Contributor

I noticed that for parameters of type INTEGER, it is allowed to enter a decimal value.
No way to block the filling of the decimal character ?

Normally the filling is blocked, but when a default is in place, the blocking does not work and I cannot find the root cause.
As the input is validated before the action is executed and then a decimal value is rejected, I don't think this is a real problem.

florian-h05 added a commit that referenced this issue Oct 30, 2024
This allows setting the step size for date and datetime contexts, which
makes the UI display seconds if the step size is 1.

Refs #2847.

Signed-off-by: Florian Hotze <dev@florianhotze.com>
florian-h05 added a commit to florian-h05/openhab-core that referenced this issue Nov 1, 2024
…me to enable seconds

Refs openhab/openhab-webui#2848.
Discussion in openhab/openhab-webui#2847 (comment).

Signed-off-by: Florian Hotze <dev@florianhotze.com>
@florian-h05
Copy link
Contributor

@lolodomo See openhab/openhab-core#4436 for seconds.

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 1, 2024

As the input is validated before the action is executed and then a decimal value is rejected, I don't think this is a real problem.

When I tested, the filled value 123,456 was not rejected but finally the action input was set to 123.

@florian-h05
Copy link
Contributor

When entering a decimal value into an integer field, the value is immediately rejected the moment I press the "." decimal point, except when entering decimals for 0 which is the default value. In that case, the value is only rejected on "execute action":

screenrecording.webm

J-N-K pushed a commit to openhab/openhab-core that referenced this issue Nov 6, 2024
…me to enable seconds (#4436)

Refs openhab/openhab-webui#2848.
Discussion in openhab/openhab-webui#2847 (comment).

Signed-off-by: Florian Hotze <dev@florianhotze.com>
@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 9, 2024

I am trying the seconds stuff in Eclipse with the current main branches. Of course I clicked on "resolve" in Eclipse. But I still see no seconds in the action pop-up.
I am testing with Firefox. But same in Chrome.

image

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 9, 2024

Good point is that for Instant and ZonedDateTime, the behaviour is now the same as LocalDateTime.

Due to the miss of seconds, the use of java.util.Date type still leads to an error.

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 9, 2024

When entering a decimal value into an integer field, the value is immediately rejected the moment I press the "." decimal point, except when entering decimals for 0 which is the default value. In that case, the value is only rejected on "execute action":

There was probably a misunderstanding or a missing information. I am in French so the decimal separator is "," and not ".".
If I enter a "." in a decimal or integer parameter, yes it is rejected and the default value is restored.
In a decimal parameter, I can enter 123,456 and the value I get in java object s 123.456. Fine.
The problem I was mentioning is that for an integer parameter I can also fill 123,456. In that case, the value I get in java object is 123. But I would expect the filling of "," to be rejected in an integer field because it is the decimal character in French.

@florian-h05
Copy link
Contributor

I am trying the seconds stuff in Eclipse with the current main branches.

What is the API response?

@florian-h05
Copy link
Contributor

I would expect the filling of "," to be rejected in an integer field because it is the decimal character in French.

Good point, need to check that.

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 9, 2024

I am trying the seconds stuff in Eclipse with the current main branches.

What is the API response?

  {
    "actionUid": "astro.testAction2",
    "label": "test2",
    "description": "Test action 2",
    "inputs": [
      {
        "name": "instantParam",
        "type": "java.time.Instant",
        "label": "instantParam parameter",
        "description": "Descr instantParam parameter",
        "required": false,
        "tags": [],
        "reference": "",
        "defaultValue": ""
      },
      {
        "name": "datetimeParam",
        "type": "java.time.LocalDateTime",
        "label": "datetimeParam parameter",
        "description": "Descr datetimeParam parameter",
        "required": false,
        "tags": [],
        "reference": "",
        "defaultValue": ""
      },
      {
        "name": "timeParam",
        "type": "java.time.LocalTime",
        "label": "timeParam parameter",
        "description": "Descr timeParam parameter",
        "required": false,
        "tags": [],
        "reference": "",
        "defaultValue": ""
      }
    ],
    "inputConfigDescriptions": [
      {
        "context": "datetime",
        "description": "Descr instantParam parameter",
        "label": "instantParam parameter",
        "name": "instantParam",
        "required": false,
        "type": "TEXT",
        "stepsize": 1,
        "readOnly": false,
        "multiple": false,
        "advanced": false,
        "verify": false,
        "limitToOptions": true,
        "options": [],
        "filterCriteria": []
      },
      {
        "context": "datetime",
        "description": "Descr datetimeParam parameter",
        "label": "datetimeParam parameter",
        "name": "datetimeParam",
        "required": false,
        "type": "TEXT",
        "stepsize": 1,
        "readOnly": false,
        "multiple": false,
        "advanced": false,
        "verify": false,
        "limitToOptions": true,
        "options": [],
        "filterCriteria": []
      },
      {
        "context": "time",
        "description": "Descr timeParam parameter",
        "label": "timeParam parameter",
        "name": "timeParam",
        "required": false,
        "type": "TEXT",
        "stepsize": 1,
        "readOnly": false,
        "multiple": false,
        "advanced": false,
        "verify": false,
        "limitToOptions": true,
        "options": [],
        "filterCriteria": []
      }
    ],
    "outputs": [
      {
        "name": "result",
        "type": "java.lang.Integer",
        "tags": [],
        "label": "Test result",
        "description": "",
        "reference": "",
        "defaultValue": ""
      },
      {
        "name": "instant",
        "type": "java.time.Instant",
        "tags": [],
        "label": "Instant",
        "description": "",
        "reference": "",
        "defaultValue": ""
      }
    ]
  },

@florian-h05
Copy link
Contributor

#2863 fixes the seconds for time & datetime params.

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 30, 2024

For the following action:

    @RuleAction(label = "test3", description = "Test action 3")
    public @ActionOutput(label = "Instant", type = "java.time.Instant") Instant testAction3(
            @ActionInput(name = "instantParam", label = "instantParam parameter", description = "Descr instantParam parameter") Instant instantParam) {
        logger.info("testAction3 {}", instantParam);
        return instantParam.plus(5, ChronoUnit.DAYS);
    }

I go this result:

image

Shall we assume that date&time are filled in local time zone in UI ?

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 30, 2024

In Firefox, the time picker is not working well, you are not only able to select few values. When I use the mouse, it moves from 00 to 04 but I can't select 03 or 05 for example.
It is working well in Chrome.

image

Edit: but I can fill the values in the input at top. Only the selection with the mouse is not possible for certain values with Firefox.

@lolodomo
Copy link
Contributor Author

lolodomo commented Nov 30, 2024

@florian-h05 : regarding my issue with using ",", I think it is fine, you can enter a "," but it is removed when you leave the field and there is a warning displayed.
In the following example, I filled 1,55 and then moved to the next field. It is truncated to 1.

image

Don't know if you changed something or if I did not see that !

@florian-h05
Copy link
Contributor

Shall we assume that date&time are filled in local time zone in UI ?

Yes, this is the assumption that was made with openhab/openhab-core#4440.

When I use the mouse, it moves from 00 to 04 but I can't select 03 or 05 for example.

I can reproduce this behaviour, but cannot fix it as we just use Framework7 components for those "wheels". This issue must be since openHAB 3.0 ...

Don't know if you changed something or if I did not see that !

I did not change that behaviour.

@lolodomo
Copy link
Contributor Author

Ok, I close the issue, I believe you fixed all what was possible ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working main ui Main UI
Projects
None yet
Development

No branches or pull requests

2 participants