NPM Resource Changed ? ? Pending

User tests: Successful: Unsuccessful:

avatar rs4231199
rs4231199
27 Mar 2021

Pull Request for Issue #32788 .

Summary of Changes

  1. Removed an unnecessary class.
  2. Made tr element part of tbody instead of body so that it inherits right styles.

Actual result BEFORE applying this Pull Request

Can be seen in the original issue

Expected result AFTER applying this Pull Request

icon-drag

Documentation Changes Required

None

avatar rs4231199 rs4231199 - open - 27 Mar 2021
avatar rs4231199 rs4231199 - change - 27 Mar 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Mar 2021
Category Administration Templates (admin) NPM Change JavaScript Repository
avatar infograf768
infograf768 - comment - 27 Mar 2021

Drone is not happy.

Would'nt it rather be something like

diff --git a/administrator/templates/atum/scss/vendor/_dragula.scss b/administrator/templates/atum/scss/vendor/_dragula.scss
index 9a5fd00..553ef42 100644
--- a/administrator/templates/atum/scss/vendor/_dragula.scss
+++ b/administrator/templates/atum/scss/vendor/_dragula.scss
@@ -7,12 +7,9 @@
   cursor: move;
   background-color: $teal;
+  display: table;
   opacity: .8;
 
-  &.table {
-    display: table;
-
-    td {
-      display: table-cell;
-    }
+  td, th {
+    background-color: $teal;
   }
 }
diff --git a/build/media_source/system/js/draggable.es6.js b/build/media_source/system/js/draggable.es6.js
index b69e987..84075c8 100644
--- a/build/media_source/system/js/draggable.es6.js
+++ b/build/media_source/system/js/draggable.es6.js
@@ -95,4 +95,6 @@
         return sibling === null || (sibling && sibling.tagName.toLowerCase() === 'tr');
       },
+
+      mirrorContainer: container,
     })
       .on('drag', () => {
@@ -100,6 +102,5 @@
       })
       .on('cloned', () => {
-        const el = document.querySelector('.gu-mirror');
-        el.classList.add('table');
+
       })
       .on('drop', () => {
avatar infograf768
infograf768 - comment - 27 Mar 2021

Also: we do have a /templates/cassiopeia/scss/vendor/_dragula.scss although I can't find where it is used.

avatar PhilETaylor PhilETaylor - test_item - 28 Mar 2021 - Tested successfully
avatar PhilETaylor
PhilETaylor - comment - 28 Mar 2021

I have tested this item successfully on 2111a1f

This fixes icon layout on dragging on other pages as well as the one initially reported.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32889.

avatar ceford ceford - test_item - 28 Mar 2021 - Tested successfully
avatar ceford
ceford - comment - 28 Mar 2021

I have tested this item successfully on 2111a1f

OK!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32889.

avatar rs4231199
rs4231199 - comment - 29 Mar 2021

@infograf768

  • &.table {
  • display: table;

.table can't be used here because it has been already defined and messes up the styling. Either we can remove the class or rename it. I chose to remove the .table. Can you explain why do you suggest keeping it?

Also: we do have a /templates/cassiopeia/scss/vendor/_dragula.scss although I can't find where it is used.

It is used by Dragula library to add the CSS to elements while they are being dragged. They provide some default CSS, but we can override it to suit our needs.

avatar infograf768
infograf768 - comment - 29 Mar 2021

@rs4231199
I’m not questionning your patch, just its formatting. Please look at the diff I posted. Minus means taking off the line, plus means adding it, indentation and order of stuff is corrected.

avatar infograf768
infograf768 - comment - 29 Mar 2021

As for

Also: we do have a /templates/cassiopeia/scss/vendor/_dragula.scss although I can't find where it is used.

I know what it is supposed to do, but I do not see any place in CASSIOPEA where it is used.

avatar rs4231199
rs4231199 - comment - 29 Mar 2021

I know what it is supposed to do, but I do not see any place in CASSIOPEA where it is used.

We only need to define it. Dragula adds it to the elements. see this.

avatar rs4231199 rs4231199 - change - 29 Mar 2021
Labels Added: NPM Resource Changed ?
avatar rs4231199
rs4231199 - comment - 31 Mar 2021

@Quy Please review the PR.

avatar ceford ceford - test_item - 31 Mar 2021 - Tested successfully
avatar ceford
ceford - comment - 31 Mar 2021

I have tested this item successfully on f33d13d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32889.

avatar rs4231199
rs4231199 - comment - 1 Apr 2021

I know what it is supposed to do, but I do not see any place in CASSIOPEIA where it is used.

@infograf768 Oh. I'm sorry I misunderstood your statement. I couldn't find any use of it in CASSIOPEIA either.

avatar infograf768 infograf768 - test_item - 1 Apr 2021 - Tested successfully
avatar infograf768
infograf768 - comment - 1 Apr 2021

I have tested this item successfully on f33d13d

We can delete the cassiopea specific scss in another pr


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32889.

avatar infograf768 infograf768 - change - 1 Apr 2021
Status Pending Ready to Commit
avatar infograf768
infograf768 - comment - 1 Apr 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32889.

avatar Fedik Fedik - close - 8 Apr 2021
avatar Fedik Fedik - merge - 8 Apr 2021
avatar Fedik Fedik - change - 8 Apr 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-04-08 14:49:07
Closed_By Fedik
Labels Added: ?
avatar Fedik
Fedik - comment - 8 Apr 2021

Thanks, good job!

Add a Comment

Login with GitHub to post a comment