Make draggable components icons more consistent

combinatorist commented 2 years ago

TL;DR: There's a small inconsistency in the UI that originally caused me a little bit of confusion/friction. I thought it'd be easy to fix, but I got in a little over my head with the frameworks. But, I'm sure someone who knows this code base could take it from here to fix it quite quickly.

In the Page "treeview to the left", there's a double-vertical-bars icon to indicate where you can grab a "page" to reorder it in the list.

That appears to be set here: https://github.com/gristlabs/grist-core/blob/e73e74b8adcc256b2125ea3a94801059c4bbc523/app/client/ui/TreeViewComponent.ts#L333 referencing an icon which I believe is ultimately defined by an SVG.

You can see an example from one of my docs in the bottom right of this picture:

Meanwhile, the widgets are dragged around by an icon to their top left, which look like 3 vertically-aligned dots. I believe this is defined as a character in the glyphicons font, although I can't seem to find the actual character when I inspect the page. But, I suspect it's set here: https://github.com/gristlabs/grist-core/blob/808aacdc522feeb11f74047e5bdaa5e58c49125a/app/client/components/ViewLayout.ts#L297

And you can see an example in the top right of this picture:

My proposal is to use the first icon for both cases, since it has less chance of getting confused with the horizontal triple dots used as a dropdown icon to the top right of widgets.

dsagal commented 2 years ago

This would indeed be useful cleanup. There is even a TODO in in ViewLayout.css, which this would resolve.

The only difficulty is switching from the older style of code (which uses CSS classes and glyphicons) to the newer style (which uses grainjs styled elements and own icons), which is what tripped you up, @combinatorist, by the sound of it. If a nudge can help you over that trip-up, I suggest replacing this line you found https://github.com/gristlabs/grist-core/blob/808aacdc522feeb11f74047e5bdaa5e58c49125a/app/client/components/ViewLayout.ts#L297 with:

cssDragIcon('DragDrop',

and define cssDragIcon at the bottom of the same file, by styling icon with CSS moved from .viewsection_drag_indicator in ViewLayout.css:

const cssDragIcon = styled(icon, `
  visibility: hidden;
  --icon-color: ${colors.slate};
  /* ...other icon styles if needed... */

  .viewsection_title:hover &.layout_grabbable {
    visibility: visible;
  }
`);

If you try this and run into any issues, let us know -- it should not be far from working...

combinatorist commented 2 years ago

Oh, wow - thanks for the tip, @dsagal! I only just noticed your response, but I'll try your suggestion out soon - hopefully this week!

yohanboniface commented 2 years ago

hi @combinatorist are you working on a fix or should we propose something ? :)

combinatorist commented 2 years ago

Sorry, I ran into complications with docker on the laptop I was going to use for this so I hadn't even started dev yet. If you have momentum/interest, go for it!

On Tue, Oct 25, 2022 at 3:11 AM Yohan Boniface @.***> wrote:

hi @combinatorist https://github.com/combinatorist are you working on a fix or should we propose something ? :)

— Reply to this email directly, view it on GitHub https://github.com/gristlabs/grist-core/issues/263#issuecomment-1290309176, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNNZS25VRMKQH6BDHS2XWLWE6W6XANCNFSM6AAAAAAQBUP6ZM . You are receiving this because you were mentioned.Message ID: @.***>

yohanboniface commented 2 years ago

cc @LouisDelbosc ? Sounds quick too :)

tayflo commented 1 week ago

This issue seems now solved to me and could be closed.

gristlabs / grist-core

Make draggable components icons more consistent #263