You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/08/18 11:18:46 UTC

[GitHub] [superset] kgabryje opened a new pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

kgabryje opened a new pull request #16322:
URL: https://github.com/apache/superset/pull/16322


   ### SUMMARY
   When user added 3 or more columns to a control that accepts only columns (such as Group by) and started reordering using drag and drop, the feature was starting to glitch after the first sorting.
   The first dragging and dropping a column to reorder worked correctly. Due to memoization, when user wanted to reorder the second time, the drop index from the first reordering was being used as a start index of the next reordering. That caused a glitch, which resulted in columns "jumping" and changing their positions incorrectly.
   This PR fixes that issue.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   Before: see linked issue
   After:
   https://user-images.githubusercontent.com/15073128/129889010-9e5c5a1e-28f7-41e2-a3b0-46bf6d446b86.mov
   
   ### TESTING INSTRUCTIONS
   0. Enable dnd feature flag
   1. Open a chart and add 3 or more columns
   2. Reorder the columns by drag and dropping and verify that it works correctly
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: fixes #16317 
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   CC @junlincc @jinghua-qa


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] henryyeh commented on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

henryyeh commented on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901482263


   🏷️ 2021.31


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] villebro commented on a change in pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

villebro commented on a change in pull request #16322:
URL: https://github.com/apache/superset/pull/16322#discussion_r691214366



##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
##########
@@ -63,15 +63,11 @@ export default function OptionWrapper(
   const ref = useRef<HTMLDivElement>(null);
   const labelRef = useRef<HTMLDivElement>(null);
 
-  const item: OptionItemInterface = useMemo(
-    () => ({
-      dragIndex: index,
-      type,
-    }),
-    [index, type],
-  );
   const [{ isDragging }, drag] = useDrag({
-    item,
+    item: {
+      type,
+      dragIndex: index,
+    },

Review comment:
       It appears we're a few versions behind `react-dnd` (now on 14; we're on 11). Apparently the deps can now be given as a param after spec:
   
   ![image](https://user-images.githubusercontent.com/33317356/129902439-a5e22a8a-0aa3-4e0c-b9d8-baba38650710.png)
   
   https://react-dnd.github.io/react-dnd/docs/api/use-drag
   
   Might be a good TODO for when we bump this the next time.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] github-actions[bot] commented on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

github-actions[bot] commented on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901089749






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] codecov[bot] edited a comment on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

codecov[bot] edited a comment on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901045835


   # [Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16322](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (191fa12) into [master](https://codecov.io/gh/apache/superset/commit/4960b5ee2bba5b933fa3d67341675a7ec4a63ba4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4960b5e) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/16322/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16322      +/-   ##
   ==========================================
   - Coverage   76.65%   76.65%   -0.01%     
   ==========================================
     Files         999      999              
     Lines       53397    53391       -6     
     Branches     6819     6818       -1     
   ==========================================
   - Hits        40933    40926       -7     
   - Misses      12227    12229       +2     
   + Partials      237      236       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `70.76% <100.00%> (-0.02%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ontrols/DndColumnSelectControl/DndFilterSelect.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZEZpbHRlclNlbGVjdC50c3g=) | `45.33% <ø> (ø)` | |
   | [...ontrols/DndColumnSelectControl/DndMetricSelect.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL0RuZE1ldHJpY1NlbGVjdC50c3g=) | `41.40% <ø> (ø)` | |
   | [...ontrols/FilterControl/AdhocFilterControl/index.jsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyQ29udHJvbC9pbmRleC5qc3g=) | `57.69% <ø> (ø)` | |
   | [.../FilterControl/AdhocFilterPopoverTrigger/index.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyUG9wb3ZlclRyaWdnZXIvaW5kZXgudHN4) | `80.76% <ø> (ø)` | |
   | [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `72.30% <ø> (ø)` | |
   | [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `42.05% <ø> (ø)` | |
   | [.../controls/DndColumnSelectControl/OptionWrapper.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL09wdGlvbldyYXBwZXIudHN4) | `85.45% <100.00%> (-0.52%)` | :arrow_down: |
   | [...d/src/explore/components/DatasourcePanel/index.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhc291cmNlUGFuZWwvaW5kZXgudHN4) | `73.95% <0.00%> (-2.05%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4960b5e...191fa12](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] github-actions[bot] commented on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

github-actions[bot] commented on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901089749


   @kgabryje Ephemeral environment spinning up at http://34.214.208.78:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] codecov[bot] commented on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

codecov[bot] commented on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901045835


   # [Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16322](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (638112e) into [master](https://codecov.io/gh/apache/superset/commit/4960b5ee2bba5b933fa3d67341675a7ec4a63ba4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4960b5e) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/16322/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16322      +/-   ##
   ==========================================
   - Coverage   76.65%   76.65%   -0.01%     
   ==========================================
     Files         999      999              
     Lines       53397    53391       -6     
     Branches     6819     6818       -1     
   ==========================================
   - Hits        40933    40926       -7     
   - Misses      12227    12229       +2     
   + Partials      237      236       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `70.76% <100.00%> (-0.02%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../controls/DndColumnSelectControl/OptionWrapper.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL09wdGlvbldyYXBwZXIudHN4) | `85.45% <100.00%> (-0.52%)` | :arrow_down: |
   | [...d/src/explore/components/DatasourcePanel/index.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhc291cmNlUGFuZWwvaW5kZXgudHN4) | `73.95% <0.00%> (-2.05%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4960b5e...638112e](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on a change in pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

kgabryje commented on a change in pull request #16322:
URL: https://github.com/apache/superset/pull/16322#discussion_r691239687



##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
##########
@@ -63,15 +63,11 @@ export default function OptionWrapper(
   const ref = useRef<HTMLDivElement>(null);
   const labelRef = useRef<HTMLDivElement>(null);
 
-  const item: OptionItemInterface = useMemo(
-    () => ({
-      dragIndex: index,
-      type,
-    }),
-    [index, type],
-  );
   const [{ isDragging }, drag] = useDrag({
-    item,
+    item: {
+      type,
+      dragIndex: index,
+    },

Review comment:
       Good to know!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] codecov[bot] edited a comment on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

codecov[bot] edited a comment on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901045835


   # [Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16322](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (638112e) into [master](https://codecov.io/gh/apache/superset/commit/4960b5ee2bba5b933fa3d67341675a7ec4a63ba4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4960b5e) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head 638112e differs from pull request most recent head 191fa12. Consider uploading reports for the commit 191fa12 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/16322/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16322      +/-   ##
   ==========================================
   - Coverage   76.65%   76.65%   -0.01%     
   ==========================================
     Files         999      999              
     Lines       53397    53391       -6     
     Branches     6819     6818       -1     
   ==========================================
   - Hits        40933    40926       -7     
   - Misses      12227    12229       +2     
   + Partials      237      236       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `70.76% <100.00%> (-0.02%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../controls/DndColumnSelectControl/OptionWrapper.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL09wdGlvbldyYXBwZXIudHN4) | `85.45% <100.00%> (-0.52%)` | :arrow_down: |
   | [...d/src/explore/components/DatasourcePanel/index.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhc291cmNlUGFuZWwvaW5kZXgudHN4) | `73.95% <0.00%> (-2.05%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4960b5e...191fa12](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on a change in pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

kgabryje commented on a change in pull request #16322:
URL: https://github.com/apache/superset/pull/16322#discussion_r691239687



##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
##########
@@ -63,15 +63,11 @@ export default function OptionWrapper(
   const ref = useRef<HTMLDivElement>(null);
   const labelRef = useRef<HTMLDivElement>(null);
 
-  const item: OptionItemInterface = useMemo(
-    () => ({
-      dragIndex: index,
-      type,
-    }),
-    [index, type],
-  );
   const [{ isDragging }, drag] = useDrag({
-    item,
+    item: {
+      type,
+      dragIndex: index,
+    },

Review comment:
       Good to know!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

kgabryje commented on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901088646


   /testenv up FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true FEATURE_ENABLE_DND_WITH_CLICK_UX=true


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje merged pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

kgabryje merged pull request #16322:
URL: https://github.com/apache/superset/pull/16322


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] codecov[bot] commented on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

codecov[bot] commented on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901045835


   # [Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16322](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (638112e) into [master](https://codecov.io/gh/apache/superset/commit/4960b5ee2bba5b933fa3d67341675a7ec4a63ba4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4960b5e) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/16322/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16322      +/-   ##
   ==========================================
   - Coverage   76.65%   76.65%   -0.01%     
   ==========================================
     Files         999      999              
     Lines       53397    53391       -6     
     Branches     6819     6818       -1     
   ==========================================
   - Hits        40933    40926       -7     
   - Misses      12227    12229       +2     
   + Partials      237      236       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `70.76% <100.00%> (-0.02%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../controls/DndColumnSelectControl/OptionWrapper.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EbmRDb2x1bW5TZWxlY3RDb250cm9sL09wdGlvbldyYXBwZXIudHN4) | `85.45% <100.00%> (-0.52%)` | :arrow_down: |
   | [...d/src/explore/components/DatasourcePanel/index.tsx](https://codecov.io/gh/apache/superset/pull/16322/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhc291cmNlUGFuZWwvaW5kZXgudHN4) | `73.95% <0.00%> (-2.05%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4960b5e...638112e](https://codecov.io/gh/apache/superset/pull/16322?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

kgabryje commented on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901088646


   /testenv up FEATURE_ENABLE_EXPLORE_DRAG_AND_DROP=true FEATURE_ENABLE_DND_WITH_CLICK_UX=true


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] villebro commented on a change in pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

villebro commented on a change in pull request #16322:
URL: https://github.com/apache/superset/pull/16322#discussion_r691214366



##########
File path: superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
##########
@@ -63,15 +63,11 @@ export default function OptionWrapper(
   const ref = useRef<HTMLDivElement>(null);
   const labelRef = useRef<HTMLDivElement>(null);
 
-  const item: OptionItemInterface = useMemo(
-    () => ({
-      dragIndex: index,
-      type,
-    }),
-    [index, type],
-  );
   const [{ isDragging }, drag] = useDrag({
-    item,
+    item: {
+      type,
+      dragIndex: index,
+    },

Review comment:
       It appears we're a few versions behind `react-dnd` (now on 14; we're on 11). Apparently the deps can now be given as a param after spec:
   
   ![image](https://user-images.githubusercontent.com/33317356/129902439-a5e22a8a-0aa3-4e0c-b9d8-baba38650710.png)
   
   https://react-dnd.github.io/react-dnd/docs/api/use-drag
   
   Might be a good TODO for when we bump this the next time.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] codecov[bot] edited a comment on pull request #16322: fix(explore): reordering columns with dnd sometimes glitching

Posted by GitBox <gi...@apache.org>.

codecov[bot] edited a comment on pull request #16322:
URL: https://github.com/apache/superset/pull/16322#issuecomment-901045835






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org