Adding Support for oneColumnModeSort Options (Custom Responsive Sort Order) by Kenny-Moore · Pull Request #966 · gridstack/gridstack.js

Kenny-Moore · 2019-01-25T23:01:37Z

…

Description

These are changes implementing the requested changes from issue #713.

oneColumnModeSort option for sorting that also respects the existing rtl property.

oneColumnModeSort: "row | row-reverse | column | column-reverse | none"

none: Sort by the order the widget's appear in the markup
row: (default) Sort first by y values (low to high) then sort widgets with the same y value by their x value (based on the rtl setting)
row-reverse: Sort first by y values (high to low) then sort widgets with the same y value by their x value (based on the rtl setting)
col: Sort first by x values (based on the rtl setting) then sort widgets with the same x value by their y value (low to high)
col-reverse: Sort first by x values (based on the rtl setting) then sort widgets with the same x value by their y value (high to low)

Please explain the changes you made here. Include an example of what your changes fix or how to use the changes: Please refer to #713 for more info

Checklist

Created tests which fail without the change (if possible)
All tests passing (npm test)
Extended the README / documentation, if necessary
I am not sure how to update the doc/README.md with the auto jsdoc generation tool. Is there anything else I need to add?

…Order) These are changes implmenting the requested changes from issue gridstack#713 in the gridstrack repo. oneColumnModeSort option for sorting that also respects the existing `rtl` property. `oneColumnModeSort: "row | row-reverse | column | column-reverse | none"` - `none`: Sort by the order the widget's appear in the markup - `row`: (default) Sort first by **y** values (low to high) then sort widgets with the same y value by their **x** value (based on the `rtl` setting) - `row-reverse`: Sort first by **y** values (high to low) then sort widgets with the same **y** value by their **x** value (based on the `rtl` setting) - `col`: Sort first by **x** values (based on the `rtl` setting) then sort widgets with the same **x** value by their **y** value (low to high) - `col-reverse`: Sort first by **x** values (based on the `rtl` setting) then sort widgets with the same **x** value by their **y** value (high to low)

adumesny

looks good overall. Not sure why the lock json changed given the project.json did not (note that we switched to yarn, so you'll need to revert/delete along the /dist files which are no longer checked in as well.

adumesny · 2019-04-01T19:59:06Z

src/gridstack.js

            width = width || _.chain(nodes).map(function(node) { return node.x + node.width; }).max().value();
            dir = dir != -1 ? 1 : -1;
-            return _.sortBy(nodes, [function(n) { return dir * (n.x + n.y * width); }]);
+            var x = dim != -1 ? 'x' : 'y';


might want to rename var x to dir1 and dir2 = confusing to have x = y

adumesny · 2019-04-01T22:37:00Z

src/gridstack.js


+        this.opts.oneColumnModeSort = this.opts.oneColumnModeSort.toLowerCase();
+        if (!String('row | row-reverse | column | column-reverse | none').includes(this.opts.oneColumnModeSort)) {
+            this.opts.oneColumnModeSort = 'row';


nice to keep original sort default. It's the most logical one really.

adumesny · 2019-04-01T22:41:34Z

src/gridstack.js

+                if (oneColumnModeSort !== 'none') {
+                    var dim = oneColumnModeSort.includes('column') ? -1 : 1;
+                    var dir = dim > 0 && self.opts.rtl ? -1 : 1;
+                    dir = oneColumnModeSort.includes('reverse') ? dir * -1 : dir;


code could use some comment explanation. rather hard to parse. for one thing you compare dim > 0, yet set value 1:-1 above so why not use those 2 values ?

adumesny · 2019-09-11T15:58:28Z

@gooeyideas "I am not sure how to update the doc/README.md with the auto jsdoc generation tool. Is there anything else I need to add?"

I believe README.md is manually updated - that's what I've been doing. Format is pretty self explanatory (and some tools like Visual Studio Code will render a pretty form of it so you can verify your tag formatting before checking it in).

I noticed that the sort logic was not accounting for cell sizes and was completely ignoring the rtl setting in column sort mode. I made the logic more robust and also pulled in and merged latest changes.

coveralls · 2019-09-13T22:28:48Z

Coverage increased (+0.007%) to 65.541% when pulling 1b97f40 on gooeyideas:develop into 01e0394 on gridstack:develop.

Kenny-Moore · 2019-09-13T22:38:12Z

Updated the original PR but I still need to go in and update the README, I will try to find some time to do that next week.

adumesny · 2019-11-18T07:13:10Z

@gooeyideas any chance you might update this PR to the current code base ? we're actively accepting review and releasing frequent builds right now...

adumesny · 2019-12-19T03:37:24Z

@gooeyideas I completely changed how we position widgets when changing column sizing (1 being the easy case really) with #1098. While this doesn't invalidate the need to have 'custom' order (dom vs row which is what I do hopefully corecly now) you may wan to check the latest behavior. And adjust your review if you still feel the need to have even more control...
having saved layouts could also mix with that you have (way for user to set those as opposed to gridstack generating them)

adumesny · 2020-01-18T21:05:58Z

@gooeyideas please update the review. I don't think we should support all the modes, but rather having 1 column use left-to-right-top-bottom (default) or DOM order (so you can specify a very different 1 column layout (ideally with hidden items as well).

The other directions (RTL for example) should be tied to a generic gravity that affects all things (drag&drop, moving) rather than one off 1column thing IMO.

I would love to give you credit for the DOM order at least.

Kenny-Moore · 2020-01-20T15:44:12Z

Sure thing, sorry for missing the last few messages. I recently discovered that my company changed some of our email rules so that github alerts were being treated as spam. I will try get around to this before the end of this week. Sorry again.

adumesny · 2020-01-20T16:36:00Z

great I look forward to it! are you ok doing just the DOM vs default row order we have now and we can add the others later or tie them to a generic gravity of the grid settings ?

I'm thinking that DOM order makes sense as way to specify a custom 1 column layout, whereas x,y are used for multi column layouts.

I've changed the code to support a robust multi column layout where we store each config (so going from 12 to 1 (or anything in between) and back would preserve the original layout, but have not worked on oneColumn yet, and was hoping to get your change in before I do.

adumesny · 2020-02-03T15:04:12Z

@gooeyideas supporting just DOM order (vs default row sorting) is getting much trickier than I expected now that oneColumnMode is just setColumn(1) to allow full editing (resize, move, insert) and better heuristics as we don't re-order dom elements during editing so changes will not last.

I may try in sandbox but punt on this until we support per column layout instead in the file format (what I create in memory).

adumesny · 2020-02-03T21:40:07Z

I didn't hear back so closing this as I ended up doing #1126 instead. Thanks for the initial work and let me know (in #713) if I missed anything

adumesny requested changes Apr 1, 2019

View reviewed changes

Kenny-Moore added 2 commits September 12, 2019 09:45

Merge remote-tracking branch 'upstream/develop' into develop

e5d281c

Updating the sort logic to work propery and added more robust tests

1b97f40

I noticed that the sort logic was not accounting for cell sizes and was completely ignoring the rtl setting in column sort mode. I made the logic more robust and also pulled in and merged latest changes.

adumesny mentioned this pull request Feb 2, 2020

OneColumnMode use setColumn(1) #1120

Merged

2 tasks

adumesny mentioned this pull request Feb 3, 2020

Custom Responsive Sort Order #1126

Merged

3 tasks

adumesny closed this Feb 3, 2020

Uh oh!

Conversation

Kenny-Moore commented Jan 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

adumesny left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adumesny Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

adumesny Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

adumesny Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

adumesny commented Sep 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Sep 13, 2019

Uh oh!

Kenny-Moore commented Sep 13, 2019

Uh oh!

adumesny commented Nov 18, 2019

Uh oh!

adumesny commented Dec 19, 2019

Uh oh!

adumesny commented Jan 18, 2020

Uh oh!

Kenny-Moore commented Jan 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adumesny commented Jan 20, 2020

Uh oh!

adumesny commented Feb 3, 2020

Uh oh!

adumesny commented Feb 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kenny-Moore commented Jan 25, 2019 •

edited

Loading

adumesny left a comment •

edited

Loading

adumesny commented Sep 11, 2019 •

edited

Loading

Kenny-Moore commented Jan 20, 2020 •

edited

Loading