Skip to content

Implement data-map attribute #61

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+254 −2
Open

Implement data-map attribute #61

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bf6cec0
Add datamap feature
Joshpolansky Jan 6, 2024
2228533
Fix deep values for classes
Joshpolansky Jan 9, 2024
6d5b7d7
add test code
Joshpolansky Jan 9, 2024
7b84ee3
Add doc string
Joshpolansky Jan 9, 2024
f85385d
Add example to doc string
Joshpolansky Jan 9, 2024
b72eaed
Support both syntaxes and update examples
Joshpolansky Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion example/HMI/src/testStandard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
var machine = new LUX.HMI();

/*
var machine = new LUX.Machine({
port: 8000,
ipAddress: '127.0.0.1',
maxReconnectCount: 5000,
});

*/
machine.testData = `<h1>Example Loaded</h1><p>Example HMI</p>`;
machine.testBool = false;
var Configuration = new LUX.HMI(()=>{


Expand Down
15 changes: 15 additions & 0 deletions example/HMI/views/main.handlebars
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,16 @@

<div data-map="{'innerHTML':{'data-var-name':'testData'}}">
<h1>Example</h1>
<p>Example HMI</p>
</div>

<input class="lux-text-value" data-var-name="input.color"/>
<input class="lux-text-value" data-var-name="input.width"/>
<input id="testInput"
data-map="{
'value':{'data-var-name':'testData'},
'style.backgroundColor':'input.color',
Copy link
Contributor

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
Contributor

@shanereetz shanereetz Jan 9, 2024 •
edited
Loading

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

'style.width':'input.width'
}"/>

{{> components}}
129 changes: 129 additions & 0 deletions src/HMI/lux-data-bind.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,73 @@ function isEqual(in1, in2) {

}

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

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

//Note: This function was pulled out of Lux
Copy link
Contributor

@shanereetz shanereetz Jan 9, 2024 •
edited
Loading

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

// so that it can be used in other places
// It is modified from the original to
// check if an object is more generic than just
// Object.prototype.toString.call(obj) === '[object Object]'
// It's possible that this could be pulled back into lux
// But that feels more risky than just leaving it here
// Since this is a new use case..
var e, startArrayIndex, type, i;

// First time through, split prop
if (typeof prop === "string") {
prop = prop.split(".");
}

if (prop.length > 1) {

// If not at bottom of prop, keep going
e = prop.shift();

// Check for array elements
startArrayIndex = e.indexOf('[');

if (startArrayIndex === -1) {
// If element does not exist, create it
if (typeof obj[e] === "undefined") {
obj[e] = {};
}
return LUX.setDeepObjectValue(obj[e], prop, value);
} else {
i = parseInt(e.substring(startArrayIndex + 1), 10);
e = e.substring(0, startArrayIndex);
// If array does not exist, create it
if (typeof obj[e] === "undefined") {
obj[e] = [];
}
// If element does not exist, create it
if ( typeof obj[e] === "undefined") {
obj[e][i] = {};
}
return LUX.setDeepObjectValue(obj[e][i], prop, value);
}

} else {

e = prop[0];

// Check for array elements
startArrayIndex = e.indexOf('[');
if (startArrayIndex === -1) {
obj[e] = value;
return obj[e];
} else {
i = parseInt(e.substring(startArrayIndex + 1), 10);
e = e.substring(0, startArrayIndex);
// If array does not exist, create it
if (typeof obj[e] === "undefined") {
obj[e] = [];
}
obj[e][i] = value;
return obj[e][i];
}
}
}

// Get attribute values
//----------------------

Expand Down Expand Up @@ -423,6 +490,63 @@ LUX.updateHide = function () {

}

LUX.queryDataMaps = function () {
LUX.elems.datamap = Array.prototype.slice.call(document.querySelectorAll('[data-map]'));

}

LUX.updateDataMaps = function () {

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....


LUX.visibleElems.datamap.forEach((el) => {
let mapping = el.dataMapObject;
if (!mapping) {
mapping = el.getAttribute('data-map');
mapping = mapping.replace(/'/g, '"');
mapping = JSON.parse(mapping);
el.dataMapObject = mapping;
}
let $element = $(el);
var localMachine = LUX.getMachine($element); // NOTE: Try this here before migrating everything else...
for (let key in mapping) {
updateParameter($element, localMachine, key, mapping[key]);
}
});
}
// find lock/unlock elems
LUX.queryLock = function () {
LUX.elems.lock = Array.prototype.slice.call(document.querySelectorAll('.lux-lock, .lux-unlock, [min-user-level-unlock]'));
Expand Down Expand Up @@ -1107,6 +1231,7 @@ LUX.observers = [];
* @property {Element[]} range
* @property {Element[]} tab
* @property {Element[]} component
* @property {Element[]} datamap
* @property {Element[]} hide
* @property {Element[]} lock
*/
Expand All @@ -1125,6 +1250,7 @@ LUX.elems = {
range: [],
tab: [],
component: [],
datamap: [],
hide: [],
lock: []
};
Expand All @@ -1143,6 +1269,7 @@ LUX.visibleElems = {
range: [],
tab: [],
component: [],
datamap: [],
hide: [],
lock: []
};
Expand All @@ -1165,6 +1292,7 @@ LUX.queryDom = function () {
LUX.queryRange();
LUX.queryTabs();
LUX.queryComponents();
LUX.queryDataMaps();
LUX.queryHide();
LUX.queryLock();
};
Expand All @@ -1179,6 +1307,7 @@ LUX.updateHMI = function () {
LUX.updateRange();
LUX.updateTabs();
LUX.updateComponents();
LUX.updateDataMaps();
LUX.updateHide();
LUX.updateLock();
LUX.updateReadGroupComms();
Expand Down