Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Block: Map #315

Closed
mtias opened this issue Mar 23, 2017 · 41 comments
Closed

Add Block: Map #315

mtias opened this issue Mar 23, 2017 · 41 comments
Labels
[Feature] Blocks Overall functionality of blocks New Block Suggestion for a new block [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@mtias
Copy link
Member

mtias commented Mar 23, 2017

Attributes

  • Alignment: None, Left, Right, Center, Full Bleed.
  • Caption.
  • Link.
  • Edit.

Markup

<!-- wp:map -->
<div>
    <iframe>
</div>
<!-- /wp -->

States

Placeholder:

map placeholder

Neutral:

map neutral

Selected:

map selected

@mtias mtias added Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Task Issues or PRs that have been broken down into an individual action to take labels Mar 23, 2017
@westonruter
Copy link
Member

Note that <iframe> is currently stripped by KSES by default in WordPress for users who cannot unfiltered_html.

@mtias
Copy link
Member Author

mtias commented Mar 23, 2017

This is a case where the html comment could be the entire markup, perhaps.

@aduth
Copy link
Member

aduth commented Mar 23, 2017

Maps are a bit problematic because at least if a major provider supported oEmbed, we could just save the link as the markup of the block, but since neither Google Maps nor OSM have public oEmbed as far as I can tell, we'd have to do something more like this to have built-in embed support given current core support:

https://gist.github.com/pwenzel/4694691

Or equivalent in some server-side block APIs for generating markup of a block from only a few attributes.

@mtias
Copy link
Member Author

mtias commented Mar 24, 2017

Yes, which is where just the comment may be desirable—it can be converted server side to whatever is needed, and updated if services modify their APIs.

@aduth
Copy link
Member

aduth commented Mar 24, 2017

it can be converted server side to whatever is needed

This is related to #104 but might be worth even a separate issue tracking how we expect to identify blocks server-side and filter their content on save. There's not much consideration for these behaviors in current block proposals which are primarily concerned with client-side behaviors.

@youknowriad
Copy link
Contributor

Since we're working on the blocks editor, I think we should think about "normalizing" the way we handle dynamic/static blocks:

Static Blocks: If we can generate the final displayed markup, than this is a static block and its markup should be added the postContent directly

Dynamic Blocks: In case of blocks where we can not save the final markup. Think "site title block" or "recent posts block" or even "embed block" or "map block" ( because we can not save the iframe due to this kses limitation ). For these blocks, the data needed to generate the final markup should not be inserted directly into the postContent, it needs to be saved as a comment attribute <!-- wp:embed url="" --> (or as post-meta).

I know we already have logic transforming oembed urls, or shortcodes from the postContent but I think we should think about generalizing these transformations into a single and unique model dynamic blocks where all the necessary attributes are stored in the exact same way (comment attributes for example)

@joyously
Copy link

generalizing these transformations into a single and unique model dynamic blocks where all the necessary attributes are stored in the exact same way (comment attributes for example)

Or like shortcodes? There is nothing wrong with shortcodes. The code works and there is a lot of content out there that already has this markup.

@mtias
Copy link
Member Author

mtias commented Mar 24, 2017

There is nothing wrong with shortcodes. The code works and there is a lot of content out there that already has this markup.

Shortcodes are not syntactically different from text, which poses a problem when parsing and requires complicated regex. Since they are also not native HTML they often end up rendered as unprocessed text in many contexts, which we want to avoid.

@azaozz
Copy link
Contributor

azaozz commented Mar 24, 2017

Since we're working on the blocks editor, I think we should think about "normalizing" the way we handle dynamic/static blocks

Yes, completely agree. We have to be very clear on what the differences are and how to handle both block types.

For these blocks, the data needed to generate the final markup should not be inserted directly into the postContent, it needs to be saved as a comment attribute (or as post-meta).

Yep, the dynamic blocks will be more like "placeholders" in post_content where the dynamically generated data will be inserted when the post is displayed. The "settings" for the block would be inside the block's "start tag". I'm not sure if using something like HTML attributes is best. Think we can provide better structure than telling the plugin authors "Just dump in there whatever you want and we will take care of it" :) Looking at past/current APIs that would be the wrong way to go.

In any case we will need more "storage options" for there. If any of the block settings should be considered "private" they will have to be stored outside post_content. I agree post meta makes the most sense, however there may be some limitations for using it for "private" data.

Another case is caching of infrequently changing dynamic data like the HTML retrieved from the oEmbeds providers.

There is nothing wrong with shortcodes.

Heh, have you seen how they work? :)

Seriously, shortcodes are acceptable when used as originally intended: as placeholders. However many plugins use (and abuse) them for many different things, unfortunately.

As @mtias mentioned above their main drawback is that they are used in HTML context but don't follow the HTML rules. That results in a lot of "bad" things: needing a ton of regex to make them "mostly work", bad UX when a plugin that adds them is disabled (as they stop working and show on the front-end), sometimes using several in a post makes page load quite slower, etc.

@ellatrix
Copy link
Member

Maps always make me wonder... If you embed one, then move around in the map, and then save, would you expect it to save like that? I think many people would (I would). Just a general thought, don't want to further complicate implementation.

@jasmussen
Copy link
Contributor

Maps always make me wonder... If you embed one, then move around in the map, and then save, would you expect it to save like that? I think many people would (I would). Just a general thought, don't want to further complicate implementation.

Good point, but aren't we limited by what Google Maps and Open Street Map and other map providers provide in their embeds here?

@aduth
Copy link
Member

aduth commented Mar 27, 2017

@iseulde Would it be enough to simply disable those controls? At least in a simple implementation.

Another point to note here is that Google Maps is a poor choice of map provider for self-hosted sites due to requirements enacted last year that all API usage, including embeds, include an API key (except grandfathered sites). Effectively, in order to use Google map's, we'd need the user to configure a Google API Console project, generate a key, and provide it somewhere in WordPress settings. Not a great UX 😬

@jasmussen
Copy link
Contributor

Another point to note here is that Google Maps is a poor choice of map provider for self-hosted sites due to requirements enacted last year that all API usage, including embeds, include an API key (except grandfathered sites). Effectively, in order to use Google map's, we'd need the user to configure a Google API Console project, generate a key, and provide it somewhere in WordPress settings. Not a great UX 😬

That's interesting. So wouldn't I be able to embed from here?

screen shot 2017-03-27 at 14 57 08

@aduth
Copy link
Member

aduth commented Mar 27, 2017

I believe copying the embed frame directly from Google works fine (except for roles implications noted earlier), but that you'd have a hard time generating the URL for that frame from a WordPress admin dashboard.

@jasmussen
Copy link
Contributor

but that you'd have a hard time generating the URL for that frame from a WordPress admin dashboard.

What if in the following screen ...

... we rephrased it to read "Paste the full embed URL with iframe here" (or something better), would that change things?

@aduth
Copy link
Member

aduth commented Mar 27, 2017

@jasmussen Yeah, that might help at least work around the API key requirement. Would still need some logic to dismantle the iframe code to its pieces (URL) and then reassemble into an iframe server-side.

@jasmussen
Copy link
Contributor

That brings on another question, is it even technically possibly for us to extract the coordinates from the Google maps embed? If not it would be hard to convert from Google maps to Open Street Map.

@joyously
Copy link

Why is a Map block a core block?

@jasmussen
Copy link
Contributor

jasmussen commented Mar 27, 2017

Why is a Map block a core block?

Maps are already supported in WordPress core.

One of the most important goals of the editor project is to have blocks to help surface "what might take shortcodes, custom HTML, or mystery meat embed discovery", as Matt put it when he announced the focuses for the year. The "mystery meat" refers to very few knowing that Maps are already supported in WordPress. By making it an option to insert as a block, we create an interface that lets users discover this, where previously it was "paste code and hope it works".

@ellatrix
Copy link
Member

I don't think maps are currently supported in WordPress core, but they are in WordPress.com. It's a good block to try though, similar to video.

@jasmussen
Copy link
Contributor

They are, insofar as if you paste the embed code you see in #315 (comment), you'll get a functional Google Map. The iframe isn't stripped out. Which means you don't need any plugins to add Google Maps to your sites. Or am I missing something?

@ellatrix
Copy link
Member

Iframes are only kept if you have proper privileges. I believe admin only?

@ellatrix
Copy link
Member

But yeah if you count iframes, everything is supported. :)

@jasmussen
Copy link
Contributor

Many blocks are likely to be "stub" blocks, to simply convey to the user that "yes, you can do that here, we recognize this code", even if WordPress hasn't done anything in particular to support it. The key being that users shouldn't have to go install plugins to do something that technically works in WordPress out of the box. That's my key take-away from the "mystery meat embed support" part.

@westonruter
Copy link
Member

Iframes are only kept if you have proper privileges. I believe admin only?

Right. If a user doesn't have unfiltered_html then they cannot add an iframe to the post content, by default. This capability is only available to admins on non-multisite installs, by default.

@ellatrix
Copy link
Member

Perhaps this map block is simply an embed alias like all the other aliases?

That's the problem. :) Google Maps doesn't support oEmbed. But like I said, maybe we could add some kind of whitelisting for iframes that would be beneficial for other embeds that only support iframes too.

@ellatrix
Copy link
Member

cc @azaozz

@mtias mtias modified the milestones: Beta, Nice To Have, Beta Jun 6, 2017
@mtias mtias added the New Block Suggestion for a new block label Jun 26, 2017
@marsjaninzmarsa
Copy link

This should still be plugin territory for me.

@StaggerLeee
Copy link

This one sounds very, very....yes very appealing. But remember, on almost all websites it is used just once on website.

Only way I personaly could see this block be justified is by saving latitude, longitude as Meta fields data. For further extending by plugin developers, Map with all markers, Markers from Category, custom Post Type, distance radius search, shop locator, etc....

@StaggerLeee
Copy link

Mybe you can add example to documentation, or make a block of it.
People did Gmap block, with some options.

https://blog.dayo.fr/2017/08/un-plugin-gutenberg-pour-ajouter-une-carte-google-statique/

Map pin is missing, and things like that.

Image of Yaktocat

@AdsonCicilioti
Copy link

Do not forget the styles in .json for maps in roadmap. See how this tool works -> https://mapstyle.withgoogle.com/

@karmatosed
Copy link
Member

As the project moves towards merge proposal, we are moving issues that aren't needed for that to projects. This doesn't mean they won't get done, they totally can and that's why we're moving to projects. This allows us to focus on the issues that have to happen to Gutenberg.

@ataylorme
Copy link

Took a stab at a Google Map block. Would love feedback if anyone here is still interested in one. https://github.com/pantheon-systems/google-map-gutenberg-block

@jasmussen
Copy link
Contributor

Love that, @ataylorme! Nice work!

This looks pretty much perfect, so as I give feedback, please parse it as you like and take what you feel is useful and ignore the rest. Looking at:

screen shot 2018-01-15 at 09 50 57

One of the design virtues of Gutenblocks is that by ensuring good defaults and surfacing input fields on the selected block itself, we make it so the user only has to visit block settings to perform advanced configuration.

Keeping that in mind I'd make two changes:

  1. I'd move the location field out from the sidebar and as a field that pops out as soon as a user selects the map block. Just like how when you click an image block, a caption field pops out, similarly when you click the map block, the location field would pop out.

  2. I'd default the block to be 100% wide, so it's responsive by default. You could then allow the user to customize this in the sidebar if they'd like. You could even use CSS wizardry to make even the height responsive, so the user only had to pick an aspect ratio.

Those are just ideas, though, and this looks good as is!

@StaggerLeee
Copy link

For some reason I cannot get this Gmap addon to work.
Using the same API key that works with ACF. Disabled ACF just to be sure API key is not called twice on page.
Switched static map off. Adress suggestion does not work, map is not shown just empty space and console gives 403 Gmap error.

@ataylorme
Copy link

@StaggerLeee the correct place to open an issue is on the repository for the Google Map Plugin. Please see the issue I opened there.

@ZebulanStanphill
Copy link
Member

I guess this issue should be removed from the Feature Complete milestone since it is not considered a necessary first release feature?

@karmatosed karmatosed removed this from the Feature Complete milestone May 19, 2018
@karmatosed
Copy link
Member

Good call @SuperGeniusZeb this was closed but thanks for the tidying up of milestones suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks New Block Suggestion for a new block [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests