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

Support for dynamic class in third party ui nodes #1240

Open
colinl opened this issue Aug 27, 2024 · 18 comments
Open

Support for dynamic class in third party ui nodes #1240

colinl opened this issue Aug 27, 2024 · 18 comments
Labels
feature-request New feature or request that needs to be turned into Epic/Story details needs-triage Needs looking at to decide what to do

Comments

@colinl
Copy link
Contributor

colinl commented Aug 27, 2024

Description

I don't know whether this is a feature request or just a request for guidance.

Server side the dashboard includes seamless support for class control. The developer merely needs to provide a config field className for the node. This results in a className field in the props sent to the node on page refresh. In addition there is a class field in props that is initially an empty string.
If a message containing msg.class or msg.ui_update.class then the dashboard automatically includes that setting in props.class.

Client side there appears to be partial support. On opening the page, both the configured className and any class passed in via a message, automatically appear in the top level widget element in the DOM, which is perfect. However, when msg.class or msg.ui_update.class are sent to the node, the DOM does not immediately update, it does not update until the page is refreshed.

So the question is, does the developer need to provide support for this, or should the dashboard core code do this automatically? If the developer needs to do something then what is it?
I tried setting props.class when a message is received but that did not cause the DOM to change.

See this forum post for background info.

Have you provided an initial effort estimate for this issue?

I am no FlowFuse team member

@colinl colinl added feature-request New feature or request that needs to be turned into Epic/Story details needs-triage Needs looking at to decide what to do labels Aug 27, 2024
@colinl
Copy link
Contributor Author

colinl commented Aug 28, 2024

I could really do with some help here, I cannot work out how to handle msg.class and msg.ui_update.class in the client side code.

If I send in msg.class and then refresh the page I see that the class is passed to the client side in props.class. I can also see that it automagically gets inserted into the top level widget element in the DOM, which is good. So, for example, for my classic gauge, if I pass in msg.class = "newclass" and refresh the page I see

<div id="nrdb-ui-widget-5c62f5086884aedd" class="nrdb-ui-widget nrdb-ui-gauge-classic newclass" style="...

If now I send in msg.class = "anotherclass" then I need somehow to replace that newclass in the DOM with anotherclass, without requiring a page refresh, but I cannot work out how to do that. I tried setting this.props.class = this.msg.class hoping that the DOM would automatically be updated but it is not updated.

So for the moment I am stuck, unable to implement dynamic classes in the node, so help would really be appreciated.

@colinl
Copy link
Contributor Author

colinl commented Aug 31, 2024

I have managed to work around the problem by the horrible hacky technique below. The problem is that msg.class or msg.ui_update.class add to the class of the outer widget element, which is not directly accessible to the widget. In order to achieve this immediately when a message is received I have had to add a ref to the outmost div in the .vue file

ref="wrapper"

then in the message handler:

            // if msg.class or msg.ui_update.class is provided then remove any previous dynamic class and replace with this one
            if (typeof msg.ui_update?.class == "string") {
                this.updateDynamicClass(msg.ui_update.class)
            }
            if ("class" in msg) {
                this.updateDynamicClass(msg.class)
            }

and

        updateDynamicClass: function (newClass) {
            // Hack to remove class added by msg.class at the outermost widget element and replace with new value
            //console.log(`outer class: ${this.$refs.wrapper.parentNode.className}`)
            const classes = this.$refs.wrapper.parentNode.className.split(' ')
            this.$refs.wrapper.parentNode.className = `${classes[0]} ${classes[1]} ${newClass}`
            //console.log(`now: ${this.$refs.wrapper.parentNode.className}`)
        },

This is obviously not ideal. If there is not currently a better way then I think something needs to be provided to the framework.

@colinl
Copy link
Contributor Author

colinl commented Sep 2, 2024

@joepavitt is there must be a better way for me to handle this?

@joepavitt
Copy link
Collaborator

I'm om holiday this week @colinl - I'll address it next week once I'm back

@colinl
Copy link
Contributor Author

colinl commented Sep 2, 2024

Great, thanks.

@colinl
Copy link
Contributor Author

colinl commented Sep 10, 2024

Perhaps the best way to move this forward would be to add class handling to the example ui node. With a field for the configuration of class, and the ability to change it dynamically. Then it will be clear what the recommended technique is.

If the example could also include the use of msg.enabled and also another user defined property config and dynamic update then that would be even better. It is still not clear to me exactly how I should handle those.

@bartbutenaers
Copy link
Contributor

@colinl
I had overlooked this question. Needed the same last week and it seemed that the dashboard handles it automatically. See my changes although there is more non class related stuff in that commit...

@colinl
Copy link
Contributor Author

colinl commented Sep 11, 2024

Thanks Bart.
With that setup, if you specify a class in the node's config, which div does it get included in?
If you specify a class dynamically does it keep the original class and add the new one? Which div does it get added into?
If you then refresh the page where does the new class appear?

@bartbutenaers
Copy link
Contributor

@colinl:
I use it here in my vue file:

<template>
    <div className="nrdb-video" :class="{'nrdb-nolabel': !label, [className]: !!className}">

So the div gets the nrdb-video class, and on top of that it gets the class name(s) specified in the config screen. These class names are all appended. If you are not sure, you can right click on your element in the browser and choose "Inspect" from the popup menu. Then you can see all the classes that are applied to your div.

You can see it in action on my wiki.

@colinl
Copy link
Contributor Author

colinl commented Sep 11, 2024

Do you know what it is that adds it immediately when it is updated dynamically? Is it the mapState stuff?

@colinl
Copy link
Contributor Author

colinl commented Sep 11, 2024

I am not seeing quite what you describe. I specified a class original in the node config and initially I see

<div id="nrdb-ui-widget-adfb6b791d2a1e38" class="nrdb-ui-widget nrdb-ui-video original" 
    style="display: grid; grid-template-columns: minmax(0px, 1fr); 
    grid-template-rows: repeat(0, minmax(var(--widget-row-height), auto)); 
    grid-column-end: span min(6, var(--layout-columns));">
  <div data-v-61ae52f6="" class="nrdb-nolabel">
    <video data-v-61ae52f6="" src= ...

Note that class nrdb-video does not appear at all and that original is in the div above the one with nrdb-nolabel

When I then inject the green border I get

<div id="nrdb-ui-widget-adfb6b791d2a1e38" class="nrdb-ui-widget nrdb-ui-video original video-green-border"
   style="display: grid; grid-template-columns: minmax(0px, 1fr); 
   grid-template-rows: repeat(0, minmax(var(--widget-row-height), auto)); 
   grid-column-end: span min(6, var(--layout-columns));">
  <div data-v-61ae52f6="" class="nrdb-nolabel"><video data-v-61ae52f6="" src= ...

It has added the border colour, but again not where I would have expected it.

I don't understand what :class="{'nrdb-nolabel': !label, [className]: !!className}" does. That appears to expand to an object rather than a list of classes.

@joepavitt
Copy link
Collaborator

Sorry it's taken so long to look at this.

the DOM does not immediately update, it does not update until the page is refreshed.

This should be the case. We have the checkDynamicProperties function which is part of our data tracker (which sets up the default event handlers for nodes). If you've got that setup, then the dynamic class stuff should just work

@joepavitt
Copy link
Collaborator

Just tried this with ui-example.

Clean State

Screenshot 2024-09-11 at 13 42 43

Injecting msg.ui_update.class = "my-class"

Screenshot 2024-09-11 at 13 43 27

Which also persists on page refresh

@colinl
Copy link
Contributor Author

colinl commented Sep 12, 2024

Thanks Joe, I didn't realise it should work out of the box. Now I need to find what it is that is missing from my node.

@joepavitt
Copy link
Collaborator

I'm not working today, but UI Example should have the answer

@colinl
Copy link
Contributor Author

colinl commented Sep 12, 2024

UI Example should have the answer

Yes, I am working through looking for what I am missing.

@bartbutenaers
Copy link
Contributor

Sorry for the late response @colinl, due to lack of time.
But I see that Joe already answered your question.
Don't forget to close this issue if it is solved :-)

@colinl
Copy link
Contributor Author

colinl commented Sep 16, 2024

I haven't done any more yet, also due to lack of time.

Did you notice my comment noting that even though you have
<div className="nrdb-video" :class="{'nrdb-nolabel': !label, [className]: !!className}">
that that nrdb-video does not appear in the DOM as far as I can see.

Also could you explain what :class="{'nrdb-nolabel': !label, [className]: !!className}" please? I don't understand what giving it an object does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request that needs to be turned into Epic/Story details needs-triage Needs looking at to decide what to do
Projects
Status: Backlog
Development

No branches or pull requests

3 participants