Snap icon indicating copy to clipboard operation
Snap copied to clipboard

Hard-coded number of categories in the GUI layout

Open ypiguet-epfl opened this issue 2 years ago • 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
            );
        }

ypiguet-epfl avatar Feb 08 '23 21:02 ypiguet-epfl

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.

jmoenig avatar Feb 08 '23 22:02 jmoenig

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

jguille2 avatar Feb 08 '23 23:02 jguille2

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?

jmoenig avatar Feb 09 '23 08:02 jmoenig

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

jguille2 avatar Feb 09 '23 08:02 jguille2

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!

jmoenig avatar Feb 09 '23 09:02 jmoenig

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

Faris90 avatar Jul 23 '23 13:07 Faris90

image

Faris90 avatar Jul 23 '23 13:07 Faris90