Feat: Convert DpTableHeader.vue to a template #205

elmasdemos · 2023-05-05T10:19:29Z

Ticket:: https://yaits.demos-deutschland.de/T19784

Description:

convert functional component DpTableHeader.vue into a template
convert ResizableColumns.js to a vue file and template
fix the error e.getBoundingClientRect() is not a function which is caused of the changes (see commit message and comment on code for the fix)

…sizableColumns component there was an error about e.getBoundingClientRect() this error exists because the dom structure of the component DpTableHeader changed so that there are not only HTML elements in tableHeader here needs to be a check if tableHeader.nodeName is 'TH'

…der_into_template

salisdemos

First of all thanks for all the effort

salisdemos · 2023-05-08T12:05:50Z

src/components/core/DpDataTable/DpDataTable.vue

+        /**
+         * The convert of the component DpTableHeader.vue from a functional component to a template indicates,
+         * that the DOM Element structure changed, so that the tableHeaders doesnt just contains HTML elements,
+         * it contains other code parts which are not HTML elements.
+         * So you get an error, that the method getBoundingClientRect is not a function,
+         * because it needs the same dom structure as before and the belonging HTML elements.
+         * So here needs to be a check if the tableHeader.nodeName is a 'th' element for this case.
+         * A general check if the tableHeader.nodeName is an HTML element here is inconvenient,
+         * so here its okay to do the check with directly with the 'th' element.
+         */
+        if(tableHeader.nodeName === "TH") {


Are You sure You have to check if its a TH? Isn't it enough to check if its a node?
if(tableHeader.nodeName) ?

And also its good to know where the bug came from right now, I think the depth of detail don't helps anyone in the feature. I suggest to shorten the comment.

And aside from that. Did You find out why There are none-TH elements in the Array?

replacing node instead of nodeName didnt work. But it works with nodeTyoe, so I changed the check this way. done 9c36b6e

the reason why there are other elements are because of the v-if structure in the template. see comment in the code

salisdemos · 2023-05-08T12:06:11Z

src/components/core/DpDataTable/DpResizableColumn.vue

@@ -0,0 +1,104 @@
+<template>
+  <th
+    :class="`c-data-table__resizable ${isLast? 'u-pr-0' : ''}`"


Suggested change

:class="`c-data-table__resizable ${isLast? 'u-pr-0' : ''}`"

class="c-data-table__resizable"

:class="{ 'u-pr-0' : isLast }"

done 4c37717

salisdemos · 2023-05-08T12:08:13Z

src/components/core/DpDataTable/DpResizableColumn.vue

+
+  methods: {
+    initResize (e, idx) {
+      this.resize = document.querySelector(`th[data-col-idx='${idx}']`)


We can make this more vue-ish and use refs instead

done 4c37717

salisdemos · 2023-05-08T12:09:31Z

src/components/core/DpDataTable/DpResizableColumn.vue

+      document.getElementsByTagName('body')[0].removeEventListener('mousemove', this.namedFunc)
+      document.getElementsByTagName('body')[0].removeEventListener('mouseup', this.stopResize)
+      document.getElementsByTagName('body')[0].classList.remove('resizing')


Nitpick to make it a bit shorter

Suggested change

document.getElementsByTagName('body')[0].removeEventListener('mousemove', this.namedFunc)

document.getElementsByTagName('body')[0].removeEventListener('mouseup', this.stopResize)

document.getElementsByTagName('body')[0].classList.remove('resizing')

document.querySelector('body').removeEventListener('mousemove', this.namedFunc)

document.querySelector('body').removeEventListener('mouseup', this.stopResize)

document.querySelector('body').classList.remove('resizing')

thanks. done 4c37717

salisdemos · 2023-05-08T12:11:54Z

src/components/core/DpDataTable/DpTableHeader.vue

+        v-if="isResizable"
+        :is-last="headerFields.length === idx"
+        :header-field="hf"
+        :idx="idx">


does v-text works here too?

v-text doesnt work in combination with slots. thanks for the question to think about and to try it

salisdemos · 2023-05-08T12:12:19Z

src/components/core/DpDataTable/DpTableHeader.vue

+        {{ hf.label }}
+      </template>
+    </th>
+    <th v-if="isTruncatable"


Suggested change

<th v-if="isTruncatable"

<th

v-if="isTruncatable"

done 853e2c7

salisdemos · 2023-05-08T12:12:30Z

src/components/core/DpDataTable/DpTableHeader.vue

+      @click="listeners.toggleWrapAll()">
+      <dp-wrap-trigger :title="translations.headerExpandHint" />
+    </th>
+    <th v-if="isExpandable"


Suggested change

<th v-if="isExpandable"

<th

v-if="isExpandable"

thanks. done 853e2c7

src/components/core/DpDataTable/DpDataTable.vue

…re double ths inside the component and outside of the component

…e nodeName

otherwise the slot doesnt know that it has a reference to the text

…der_into_template # Conflicts: # CHANGELOG.md

spiess-demos

lgtm (code review)

thanks for taking this on!

salisdemos

just two noneblocking nitpicks

salisdemos · 2023-05-09T09:04:43Z

src/components/core/DpDataTable/DpResizableColumn.vue

+
+  computed: {
+    isResizableColumn () {
+      return hasOwnProp(this.headerField, 'resizeable')? this.headerField.resizeable : true


Suggested change

return hasOwnProp(this.headerField, 'resizeable')? this.headerField.resizeable : true

return hasOwnProp(this.headerField, 'resizeable') ? this.headerField.resizeable : true

done 7ea8c56

salisdemos · 2023-05-09T09:07:17Z

src/components/core/DpDataTable/DpResizableColumn.vue

+        const newNextWidth = this.nextWidth - mouseMoved
+        if (newWidth > 25 && newNextWidth > 25) {


Suggested change

const newNextWidth = this.nextWidth - mouseMoved

if (newWidth > 25 && newNextWidth > 25) {

const newNextWidth = this.nextWidth - mouseMoved

if (newWidth > 25 && newNextWidth > 25) {

done 7ea8c56

add line break if new if condition add space before the question mark in ternary

…der_into_template

elmasdemos added 3 commits May 5, 2023 12:07

WIP convert dptableheader to a template

65febd5

WIP convert dptableheader to a template add scope to th

cc70739

Remove doubled th

22459e5

elmasdemos requested review from salisdemos and muellerdemos May 5, 2023 10:19

elmasdemos self-assigned this May 5, 2023

elmasdemos added 5 commits May 5, 2023 15:01

WIP change logic because of if last column and due to resizable prop

3abb458

ref class should be in the tr tag

f26b842

Merge remote-tracking branch 'origin/main' into feat_convert_tablehea…

f5a7029

…der_into_template

Add the change of DpTableHeader in changelog.md

0ff4b74

elmasdemos requested review from spiess-demos and removed request for muellerdemos May 8, 2023 11:40

elmasdemos added enhancement New feature or request review:frontend labels May 8, 2023

elmasdemos marked this pull request as ready for review May 8, 2023 11:41

elmasdemos changed the title ~~Feat: convert tableheader into template~~ Feat: convert DpTableHeader.vue to a template May 8, 2023

elmasdemos changed the title ~~Feat: convert DpTableHeader.vue to a template~~ Feat: Convert DpTableHeader.vue to a template May 8, 2023

salisdemos requested changes May 8, 2023

View reviewed changes

spiess-demos reviewed May 8, 2023

View reviewed changes

src/components/core/DpDataTable/DpDataTable.vue Outdated Show resolved Hide resolved

elmasdemos added 10 commits May 8, 2023 16:12

Remove one th because its changing the style of the column if there a…

7a6b8d5

…re double ths inside the component and outside of the component

Checking what shows note and what shows nodeName

0962b14

Checking what shows note and what shows nodeName

5248734

Checking what nodeType shows

88166ee

change the condition and use the check for the nodeType instead of th…

11f602c

…e nodeName

Do the check with nodeType instead of nodeName, shorten the comment

9c36b6e

Code beautify

4c37717

Code beautify, set correct indentation, use v-text

853e2c7

Remove wrong component

b0d4f0f

wip looking for the error

1f00cda

elmasdemos added 2 commits May 9, 2023 09:13

wip looking for the error

125f8d6

Remove v-text and write it back in the old place because

5611333

otherwise the slot doesnt know that it has a reference to the text

elmasdemos requested review from salisdemos and spiess-demos May 9, 2023 08:40

Merge remote-tracking branch 'origin/main' into feat_convert_tablehea…

b794e3c

…der_into_template # Conflicts: # CHANGELOG.md

spiess-demos approved these changes May 9, 2023

View reviewed changes

salisdemos approved these changes May 10, 2023

View reviewed changes

elmasdemos added 3 commits May 15, 2023 13:41

Code beautify

7ea8c56

add line break if new if condition add space before the question mark in ternary

Merge remote-tracking branch 'origin/main' into feat_convert_tablehea…

5aa3925

…der_into_template

Merge remote-tracking branch 'origin/main' into feat_convert_tablehea…

cfdb0f7

…der_into_template

elmasdemos merged commit 1eccbcd into main May 15, 2023

elmasdemos deleted the feat_convert_tableheader_into_template branch May 15, 2023 12:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Convert DpTableHeader.vue to a template #205

Feat: Convert DpTableHeader.vue to a template #205

elmasdemos commented May 5, 2023 •

edited

Loading

salisdemos left a comment

salisdemos May 8, 2023

elmasdemos May 9, 2023

elmasdemos May 9, 2023

salisdemos May 8, 2023

elmasdemos May 9, 2023

salisdemos May 8, 2023

elmasdemos May 9, 2023

salisdemos May 8, 2023

elmasdemos May 9, 2023

salisdemos May 8, 2023

elmasdemos May 9, 2023

salisdemos May 8, 2023

elmasdemos May 9, 2023

salisdemos May 8, 2023

elmasdemos May 9, 2023

spiess-demos left a comment

salisdemos left a comment

salisdemos May 9, 2023

elmasdemos May 15, 2023

salisdemos May 9, 2023

elmasdemos May 15, 2023

	:class="`c-data-table__resizable ${isLast? 'u-pr-0' : ''}`"
	class="c-data-table__resizable"
	:class="{ 'u-pr-0' : isLast }"

	return hasOwnProp(this.headerField, 'resizeable')? this.headerField.resizeable : true
	return hasOwnProp(this.headerField, 'resizeable') ? this.headerField.resizeable : true

		const newNextWidth = this.nextWidth - mouseMoved
		if (newWidth > 25 && newNextWidth > 25) {

Feat: Convert DpTableHeader.vue to a template #205

Feat: Convert DpTableHeader.vue to a template #205

Conversation

elmasdemos commented May 5, 2023 • edited Loading

Description:

salisdemos left a 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

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

spiess-demos left a comment

Choose a reason for hiding this comment

salisdemos left a 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

elmasdemos commented May 5, 2023 •

edited

Loading