DigitalCommons / mykomap

A web application for mapping initiatives in the Solidarity Economy
3 stars 0 forks source link

Pop-up dialog size cannot be set using query paramters #208

Closed wu-lee closed 1 year ago

wu-lee commented 1 year ago

There is a configuration setting for the dialog size, as per the CONFIG.md documentation:

dialogueSize

  • type: {DialogueSize} An object containing only string values.
  • in string context: A comma-delimited list of name-value pairs, each delimited by a colon. Therefore no commas or colons can exist in either names or values. Spaces are not trimmed, and later key duplicates will overwrite earlier ones.
  • default:
    {
    "width": "35vw",
    "height": "225px",
    "descriptionRatio": 2.5
    }
  • settable?: yes

Set the dimensions of the dialogue box. Height and width are raw css values descriptionRatio is how many times larger the description section is than the contact section. These values are used in view/map.js

This appears to work correctly if set in the config supplied on building the map, but not if it is supplied as a query parameter, e.g.:

http://example.com/?dialogueSize={width:"35vw",height:"12vw"}

The values height/width inserted into the CSS in this case seem to be "undefined" which suggests that something is broken.

wu-lee commented 1 year ago

Ok, correction: works as designed. Or mostly. I just misread the instructions.

The URL above should look like this:

http://example.com/?dialogueSize=width:35vw,height:12vw

Those comma-delimited parameters are meant to be optional. But if they're missing they parse badly as NaN. The above ends up setting the descriptionRatio to NaN (basically a bug). However, in practise these invalid CSS values are ignored, so it's not one that manifests as a bug.

If you want to set the descriptionRatio, you'd do like this (the default is 2.5 when this config is entirely missing).

http://example.com/?dialogueSize=width:35vw,height:12vw,descriptionRatio:1

That works, and sets the left and right components of the dialog to be equal in size. (1:1 - the descriptionRatio sets the first number in that ratio).

Arguably though, there should be some validation and a warning somewhere if they're invalid. This is generally true of all the configs.

ColmDC commented 1 year ago

Arguably though, there should be some validation and a warning somewhere if they're invalid. This is generally true of all the configs.

I think there is a ticket already for that https://github.com/DigitalCommons/mykomap/issues/166

ColmDC commented 1 year ago

Arguably though, there should be some validation and a warning somewhere if they're invalid. This is generally true of all the configs.

I think there is a ticket already for that #166

Actually that is not the one, but it is related. Correct one is https://github.com/DigitalCommons/mykomap/issues/179

ColmDC commented 1 year ago

Did you check if the units for height were correct? If I set a a vertical height to 50vw, I expect it would be 1/2 the size of the screen real estate, which is how the width parameters work when using vw.

ColmDC commented 1 year ago

Also is the height a max or should it reduce in size to content? https://github.com/DigitalCommons/dotcoop-project/issues/38#issuecomment-1597699405

ColmDC commented 1 year ago

@wu-lee Have you looked at how the panning works when selecting an inititaitv/event. It seems ot always centre it in the viewport, which works well for small dialogs. I wonder is making that more intellegint easier than making the dialog resizing more intelligent? Essentially vertically cnetering the dialog by lowering the marker postion?

wu-lee commented 1 year ago

Did you check if the units for height were correct? If I set a a vertical height to 50vw, I expect it would be 1/2 the size of the screen real estate, which is how the width parameters work when using vw.

Not sure what you mean exactly? The units need to be included in the parameters - so use 50vw, not just 50. But assuming you do that correctly, the browser stylesheet interprets the units. Note that 50vw is 50% of the screen width, is that what you were expecting?

wu-lee commented 1 year ago

@wu-lee Have you looked at how the panning works when selecting an initiative/event. It seems to always centre it in the viewport, which works well for small dialogs. I wonder is making that more intelligent easier than making the dialog resizing more intelligent? Essentially vertically centring the dialog by lowering the marker position?

It horizontally centres it (left-right), yes. I think that's simple to do because you just take the pin coordinates and centre that, which centres the dialog because it is horizontally symmetrical around the pin.

Whereas vertically - you can't use that simple rule, as it wouldn't centre the pop-up, because it isn't vertically symmetric around the pin. It sits above the pin. So you'd need to find the centre of the pop-up dialog. That's possible but would take more faff, and I wonder if leaflet exposes these details to you.

Ideally leaflet should be able to do this by itself I think...

wu-lee commented 1 year ago

Also is the height a max or should it reduce in size to content? DigitalCommons/dotcoop-project#38 (comment)

See my response to that comment in that thread. Short answer: the dialog height is fixed, not a maximum. So it won't shrink if there's not much content. This is a CSS limitation. What it does do is add scrollbars if the content overflows.

ColmDC commented 1 year ago

Did you check if the units for height were correct? If I set a a vertical height to 50vw, I expect it would be 1/2 the size of the screen real estate, which is how the width parameters work when using vw.

Not sure what you mean exactly? The units need to be included in the parameters - so use 50vw, not just 50. But assuming you do that correctly, the browser stylesheet interprets the units. Note that 50vw is 50% of the screen width, is that what you were expecting?

I just got confused by your example.

http://example.com/?dialogueSize=width:35vw,height:12vw,descriptionRatio:1

If I use height:12vh it works as expected! :-)

ColmDC commented 1 year ago

Please add corrected example to the docs https://github.com/DigitalCommons/mykomap/blob/master/CONFIG.md#dialoguesize

and you cam close.

ColmDC commented 1 year ago

That's possible but would take more faff, and I wonder if leaflet exposes these details to you.

Okay, lets leave that for now. Maybe add to Marcel's design spec...

wu-lee commented 1 year ago

Please add corrected example to the docs https://github.com/DigitalCommons/mykomap/blob/master/CONFIG.md#dialoguesize

and you cam close.

I've added some improvements. The results can be seen on the dev branch:

https://github.com/DigitalCommons/mykomap/blob/dev/CONFIG.md#dialoguesize