Feature/javascript improvements 2 #649

MikeVanMourik · 2024-07-31T16:08:47Z

Describe your changes

upgrading js to use more modern methods

Type of change

Please check only ONE option.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Upgrade

How was this tested?

ran locally and comparing it with the wiserdemo on the side (as far as i can test on that since not everything on the demo site seems to work)

Checklist before requesting a review

I have reviewed and tested my changes
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I selected develop as the base branch and not main, or the pull request is a hotfix that needs to be done directly on main
I double checked all my changes and they contain no temporary test code, no code that is commented out and no changes that are not part of this branch
I added new unit tests for my changes if applicable

Related pull requests

Link to Asana ticket

https://app.asana.com/0/1204371236183532/1205853947344178/f

GilianJuice · 2024-08-01T11:42:49Z

Api/Modules/Items/FieldTemplates/image-upload.js

The kendoSortable component still has several functions that you didn't convert to arrow functions. It also uses Wiser.api().then() and .catch(), which could be converted to using async and await.

GilianJuice · 2024-08-01T11:50:51Z

Api/Modules/Items/FieldTemplates/item-linker.js

-            $(this).parents("ul, li").each(function () {
-                var treeView = checkTreeElement.data("kendoTreeView");
+        checkTreeElement.find(".k-in:contains(" + filterText + ")").each(() => {
+            $(this).parents("ul, li").each( () => {


Remember that changing functions to arrow functions also changes how the this keyword works in javascript. You forgot to change the code to handle this, so this code will no longer work now. The code $(this) will now return something else, because it's inside an arrow function.

Before it was an arrow function, this was the HTML element, now it's the class/object that the code is located in. To fix this, you change the function; The each function of jQuery has 2 paramers: index and element, you can use those instead of the this keyword. Like this:

checkTreeElement.find(`.k-in:contains(${filterText})`).each((checkboxIndex, checkboxElement) => { $(checkboxElement).parents("ul, li").each((listIndex, listElement) => { const treeView = checkTreeElement.data("kendoTreeView"); treeView.expand($(listElement).parents("li")); $(listElement).show(); }); });

(You also forgot to use a string template here, see my example.)

GilianJuice · 2024-08-01T11:51:58Z

Api/Modules/Items/FieldTemplates/item-linker.js

+            $(this).parents("ul, li").each(() => {
                $(this).show();


Same here with the this keyword.

GilianJuice · 2024-08-01T11:54:40Z

Api/Modules/Items/FieldTemplates/item-linker.js

@@ -123,8 +124,8 @@ Wiser.api({
    url: window.dynamicItems.settings.serviceRoot + "/GET_COLUMNS_FOR_LINK_TABLE?linkTypeNumber=" + (options.linkTypeNumber || "") + "&id=" + encodeURIComponent(currentItemId),


Please also refactor this string concatenation into a template string.

GilianJuice · 2024-08-01T11:54:57Z

Api/Modules/Items/FieldTemplates/item-linker.js

        dataSource: {
            transport: {
-                read: function(readOptions) {
+                read: (readOptions) => {
                    startLoader();

                    Wiser.api({
                        url: window.dynamicItems.settings.serviceRoot + "/GET_DATA_FOR_FIELD_TABLE?itemId=" + encodeURIComponent("{itemIdEncrypted}") + "&linkTypeNumber=" + (options.linkTypeNumber || "") + "&moduleId=" + options.moduleId + "&entity_type=" + encodeURIComponent((!options.entityTypes ? "" : options.entityTypes.join())),


Please also refactor this string concatenation into a template string.

GilianJuice · 2024-08-01T14:05:29Z

Api/Modules/Items/FieldTemplates/sub-entities-grid.js

@@ -393,8 +399,8 @@ async function generateGrid(data, model, columns) {
                                data: JSON.stringify(transportOptions.data)
                            }).then(function (customQueryResults) {


You forgot to change this to an arrow function.

GilianJuice · 2024-08-01T14:06:31Z

Api/Modules/Items/FieldTemplates/sub-entities-grid.js

            },
            cursor: "move",
-            placeholder: function (element) {
+            placeholder: (element) => {
                return element.clone().addClass("k-state-hover").css("opacity", 0.65);
            },
            container: "#overviewGrid{propertyIdWithSuffix}",
            filter: ">tbody >tr",
            change: function (e) {


You forgot to change this into an arrow function.

GilianJuice · 2024-08-01T14:06:45Z

Api/Modules/Items/FieldTemplates/textbox.js

@@ -1,9 +1,9 @@
 (function() {


You forgot to change this into an arrow function.

GilianJuice · 2024-08-01T14:06:59Z

Api/Modules/Items/FieldTemplates/timeline.js

@@ -1,15 +1,16 @@
 (function () {


You forgot to change this into an arrow function.

GilianJuice · 2024-08-01T14:07:56Z

Api/Modules/Items/FieldTemplates/timeline.js

-                field.on("click", ".openDetails", function (event) {
-                    var itemId = $(this).closest(".timelineEvent").data("itemid");
+                field.on("click", ".openDetails", (event) => {
+                    let itemId = $(this).closest(".timelineEvent").data("itemid");


The this keyword doesn't work the same anymore in an arrow function. This line should be changed to this:

let itemId = $(event.currentTarget).closest(".timelineEvent").data("itemid");

MikeVanMourik · 2024-10-10T11:42:03Z

this pull request is a lot of work since im making changes, testing and because of only getting a little bit of time for it also includes rebases, so to make things less confusing ill place this PR into Drafts. ill take it out when its ready for another review round.

GilianJuice requested changes Aug 1, 2024

View reviewed changes

MikeVanMourik force-pushed the feature/javascript-improvements-2 branch from 984e378 to 135791d Compare October 9, 2024 07:50

MikeVanMourik marked this pull request as draft October 10, 2024 11:42

MikeVanMourik added 9 commits October 25, 2024 14:29

improve wiser javascripts

eaee61b

further adjustments

501f025

fixes

739b737

image upload js changes

3fc29f4

imrpovements

9ea2a97

wip

4c0471f

fix

6af0f20

fixes

f57d67a

wip

7aca58b

MikeVanMourik force-pushed the feature/javascript-improvements-2 branch from 135791d to 7aca58b Compare October 25, 2024 12:30

wip

9c13071

This was referenced Jan 15, 2025

perform javascript improvements/cleanup in qr.js #824

Merged

perform javascript improvements/cleanup in radiobutton #825

Merged

This was referenced Mar 19, 2025

perform javascript fixes on linked-item.js #881

Merged

refactor javascript for scheduler #882

Open

refactor javascript for secure-input #883

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/javascript improvements 2 #649

Feature/javascript improvements 2 #649

MikeVanMourik commented Jul 31, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

GilianJuice Aug 1, 2024

MikeVanMourik commented Oct 10, 2024

		@@ -123,8 +124,8 @@ Wiser.api({
		url: window.dynamicItems.settings.serviceRoot + "/GET_COLUMNS_FOR_LINK_TABLE?linkTypeNumber=" + (options.linkTypeNumber \|\| "") + "&id=" + encodeURIComponent(currentItemId),

		@@ -393,8 +399,8 @@ async function generateGrid(data, model, columns) {
		data: JSON.stringify(transportOptions.data)
		}).then(function (customQueryResults) {

Feature/javascript improvements 2 #649

Are you sure you want to change the base?

Feature/javascript improvements 2 #649

Conversation

MikeVanMourik commented Jul 31, 2024

Describe your changes

Type of change

How was this tested?

Checklist before requesting a review

Related pull requests

Link to Asana ticket

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeVanMourik commented Oct 10, 2024