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

Implement data-map attribute #61

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Implement data-map attribute #61

wants to merge 6 commits into from

Conversation

Joshpolansky
Copy link
Member

@Joshpolansky Joshpolansky commented Jan 9, 2024

What

Implement a new attribute "data-map" that can have a complex json object to map variables to values on an object.

Why

This allows the user to specify more complex databinding than just value. As we use more web components, this will allow us to easily integrate them

Open question:

Currently this writes to the objects properties, but does not set the attributes.. do we want a way to do one or the other? I could image have a prefix on the value to set to specify if it's an attribute or property

I might propose that an @ prefix means attribute?

Example useage:

<!-- This is just normal data binding writing to local data -->
<input class="lux-text-value" data-var-name="input.color"/>
<input class="lux-text-value" data-var-name="input.width"/>

<!-- This is new data-map writing local data to arbitrary properties-->
<input id="testInput" 
data-map="{
    'value':{'data-var-name':'testData'}, 
    'style.backgroundColor':'input.color',
    'style.width':'input.width',
    '@myAttribute':'input.attributevalue'
    }"/>

<!-- This is new data-map writing local data to arbitrary properties-->
<div data-map="{'innerHTML':{'data-var-name':'testData'}}">
    <h1>Example</h1>
    <p>Example HMI</p>
</div>

<ob-rudder-medium data-map="{ 'rudderAngle' : 'tmplitTest:tmplit.Slider' , ' rudderSetPointAngle' : 'tmp1itTest:tmplit.Numericlnput' }"/>

AD9F4DEE-B89E-4466-8AA2-454A1AADD33E

@Joshpolansky Joshpolansky linked an issue Jan 9, 2024 that may be closed by this pull request
@shanereetz
Copy link
Member

Would you provide a small usage example?

@@ -96,6 +96,73 @@ function isEqual(in1, in2) {

}

// Set a value deep within an arbitrary object
LUX.setDeepObjectValue = function (obj, prop, value) {
//Note: This function was pulled out of Lux
Copy link
Member

@shanereetz shanereetz Jan 9, 2024

Choose a reason for hiding this comment

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

Could you clarify the meaning of "this function was pulled out of LUX"? Maybe it was pulled from a separate file in the repo? I find it confusing since this still is in the LUX repo.

Also, it is unclear why this is "risky"

Copy link
Member Author

Choose a reason for hiding this comment

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

there is a setDeepValue in Lux.js. I didn't want to modify that because i don't know if there are ramifications to the way it was functioning

@@ -96,6 +96,73 @@ function isEqual(in1, in2) {

}

// Set a value deep within an arbitrary object
LUX.setDeepObjectValue = function (obj, prop, value) {
Copy link
Member

Choose a reason for hiding this comment

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

Requesting to add a docstring for what these arguments signify

Comment on lines 500 to 533
function updateParameterValue($element, localMachine, key, dataVarName) {
let keyname = key.replace('.','-')
if ($element.attr('data-var-name-added-' + keyname) != dataVarName) {
$element.attr('data-var-name-added-' + keyname, dataVarName)
localMachine.initCyclicReadGroup(LUX.getDataReadGroup($element), dataVarName);
}
let value = localMachine.value(dataVarName);
if ($element.attr('data-machine-value-' + keyname) != value) {
$element.attr('data-machine-value-' + keyname, value)
$element.each((index, val) => {
LUX.setDeepObjectValue(val, key, value);
});
}
}

function updateParameter($element, elMachine, key, value) {
if (typeof value === 'string') {
updateParameterValue($element, elMachine, key, value);
return
}

if (typeof value === 'object' && value['data-var-name']) {
let {
['data-var-name']:dataVarName,
machine
} = value;
let localMachine = window[machine]
if(localMachine == undefined){
localMachine = elMachine;
}
updateParameterValue($element, localMachine, key, dataVarName);
return
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Declaring these everytime the function is called feels wasteful

Copy link
Member Author

Choose a reason for hiding this comment

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

ya, i was thinking that too, but i also don't want to pollute, where should they go?

Copy link
Member

Choose a reason for hiding this comment

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

You could declare them on the function LUX.updateDataMaps.updateParameters = function....

<input id="testInput"
data-map="{
'value':{'data-var-name':'testData'},
'style.backgroundColor':'input.color',
Copy link
Member

Choose a reason for hiding this comment

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

Since the focus of the PR is on working with web components, can you tell me more about the choice to make this custom mapping attribute, vs leveraging slots on web components?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how slots are relevant, what do you mean? My understanding is that slots are used when you want to integrate user supplied html into the web component, but this is about setting values on the web component

Here is an example usage with a web component:

<ob-rudder-medium  data-map="{ 'rudderAngle' : 'tmplitTest:tmplit.Slider' , ' rudderSetPointAngle' : 'tmp1itTest:tmplit.Numericlnput' }"/>

Copy link
Member

@shanereetz shanereetz Jan 9, 2024

Choose a reason for hiding this comment

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

Slots could be relevant for uses like input forms, because (I think) the slot could just be one part of the component, with the binding already applied.

<my-fancy-input>
    <slot name="userInput" data-var-name="PLC:varToWriteTo" />
    <slot name="otherThing" data-var-name="OtherVar"/>
</my-fancy-input>

But on second thought, what is even more relevant than slots is custom attributes on web components. I believe with these you could expose certain component-specific parameters, rather than using an all-in-one mapping. I think it would be much more readable / usable, but would perhaps limit which attributes you could bind to.

Loosely-psuedocoded-example:
<od-rudder-medium data-rudder="tmplitTest:tmplit.Slider" data-rudder-angle="tmplit1Test.NumericInput"/>

The biggest downside I see to the mapping, is readability and how easily one could misspell part of the mapping, and then break all of the mapping for that component.

Copy link
Member Author

Choose a reason for hiding this comment

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

The main use case is web-components that we don't build. This is mainly useful for third party components, and means we don't have to consider how the data will map when creating components

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the od-rudder-medium example is from open-bridge

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see why it requires internal knowledge of the components, the normal use case would likely not use deep structures. You just have to know what properties to set, which would be the case with any solution.

I don't think we should build data binding into our components, IMO they should really be standalone and not consider integration into lux. The data-map solution covers all use cases, although I agree the json string is not ideal.

I was considering something like

data-lux-[arbitrary property] = "my.pv.name" but I'm not sure this is actually very good from an implementation standpoint, because there aren't great built-in ways to get data-lux* for all elements although that might not be correct.

Can you give any examples of more dev-friendly apis?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just tried using document.querySelectorAll('[data-lux*]') and it throws an error that it isn't a valid selector

Copy link
Member Author

Choose a reason for hiding this comment

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

The only way I see that we could do it is with something like

let elements = Array.from(document.querySelectorAll('*')).filter(el => 
    Array.from(el.attributes).some(attr => attr.name.startsWith('data-lux'))
);

but that is really inefficient

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested this and it's ~100x slower

Copy link
Member Author

Choose a reason for hiding this comment

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

it looks like attributes aren't case sensitive (they are always lower case) so we can't actually support properties with this

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.

Add support for arbitrary data mapping
3 participants