jmoenig/Snap

Hard-coded number of categories in the GUI layout

Opened this issue · 7 comments

When more categories are added to SpriteMorph.prototype.categories in file objects.js, corresponding buttons are not visible in the GUI because the layout assumes 4 rows in function fixCategoriesLayout in file gui.js.

Suggestion: in function fixCategoriesLayout, instead of the hard-coded constant 8, 7 and 4 in function fixCategoriesLayout, define variables

var nCatButtons = (
    SpriteMorph.prototype.categories.length
        - (contains(SpriteMorph.prototype.categories, 'lists') ? 1 : 0)
        - (contains(SpriteMorph.prototype.categories, 'other') ? 1 : 0)
);
var nCatButtonRows = Math.ceil(nCatButtons / 2);

Replace

        myself.categories.children.forEach((button, i) => {
            row = i < 8 ? i % 4 : i - 4;
            col = (i < 4 || i > 7) ? 1 : 2;
            button.setPosition(new Point(
                l + (col * xPadding + ((col - 1) * buttonWidth)),
                t + ((row + 1) * yPadding + (row * buttonHeight) + border) +
                    (i >= nCatButtons  ? border + 2 : 0)
            ));
        });

with

        myself.categories.children.forEach((button, i) => {
            row = i < nCatButtons ? i % nCatButtonRows : i - nCatButtonRows;
            col = (i < nCatButtonRows || i >= nCatButtons) ? 1 : 2;
            button.setPosition(new Point(
                l + (col * xPadding + ((col - 1) * buttonWidth)),
                t + ((row + 1) * yPadding + (row * buttonHeight) + border) +
                    (i > 7 ? border + 2 : 0)
            ));
        });

and

        } else {
            myself.categories.setHeight(
                (4 + 1) * yPadding
                    + 4 * buttonHeight
                    + (more ?
                        (more * (yPadding + buttonHeight) + border + 2)
                            : 0)
                    + 2 * border
            );
        }

with

        } else {
            myself.categories.setHeight(
                (nCatButtonRows + 1) * yPadding
                    + nCatButtonRows * buttonHeight
                    + (more ?
                        (more * (yPadding + buttonHeight) + border + 2)
                            : 0)
                    + 2 * border
            );
        }

Nice! Sounds like a good idea, thanks. I'm currently taking care of a bunch of other matters but promise to come back to these suggestions, so please don't get impatient, I appreciate this idea.

Hi!
I only want to mark this issue to track it and those possible related changes.

Note that all Snap! extensions are living with this "hard-coded numbers" and are adapted to it and so, we will have to readapt these changes. That hardcoded numbers already changed sometime ago (there were by columns and now are by rows) ...

No problem here.. only to track it, and to take care on this.

Joan

Oh, good point, @jguille2. Let's keep that in mind, that changing this will affect all forks that try to keep up with changes, especially Snap4Arduino, Turtlestitch, Beetleblocks and Netsblox. What you think about the proposed change, Joan? Would it make your life any easier?

Hi Jens (@jmoenig )
I think you have to choose the better for Snap! and the extensions will already adapt. You just have to be careful with the tempos. For example, they would be changes to make when we have been in "dev" for quite some time, and not a quick change that will be in production tomorrow.

I haven't had time to watch it much. Usually what needs to be hacked the most are pairings. For example, if an extension only adds a new category, it usually compensates the pairs by taking out the "Other" category. It's the only one that forces you to hack more than you'd like…

But I insist that the best solution for Snap! it will be the best for the entire ecosystem.

Thanks Jens

Okay, thanks @jguille2 . So, to sum up I agree with you that if we decide to make these suggested changes we don't do so on the fly in a minor patch, but in preparation for a minor or even major release with a couple of weeks or even months in dev. Sounds like a good plan!

image
@ypiguet-epfl this happens though if you make a custom category via snap