Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 40 additions & 5 deletions docs/plugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ The capability a plugin provides determines *how* its frontend renders:
**Marker fields** (``MarkerFieldPluginBase``)
Render a single location field inside a marker popup (capability ``"MarkerField"``,
mounted by ``FieldRenderer``). ``FieldRenderer`` renders a field as a **pipe**: the raw
value flows through a chain of stages — the built-in for the field ``type`` (e.g.
``hyperlink``/``CTA``) renders it, then each field plugin attached to that ``type``
transforms the result. A plugin's ``config`` declares which field it attaches to and
value flows through a chain of stages — the server-rendered ``html`` for the field seeds
it, then each field plugin attached to that ``type`` transforms the result. A plugin's ``config`` declares which field it attaches to and
where it sits:

- ``field``: the field ``type`` it applies to. For a custom type, the plugin's platzky
Expand Down Expand Up @@ -108,6 +107,41 @@ Field plugins
value flows through the built-in for the field ``type`` (if any) and then each field plugin
attached to that ``type`` via ``config.field``, innermost-first by ``config.order``.

.. _plugins-shortcode-rendered-fields:

**A platzky plugin needs no goodmap frontend at all.** When a field name matches a shortcode
contributed by a loaded platzky plugin, ``prepare_pin`` also calls that shortcode's
``render_value`` and carries the result as ``html`` on the field value. If no first-party
renderer claims the ``type``, ``FieldRenderer`` seeds the fold with that HTML — so a plugin
displays correctly by shipping a Python shortcode alone: no Module Federation build, no
bundle to serve, no ``config.field`` to keep in sync. Field plugins below remain the way to
add behaviour goodmap's own React tree must participate in, and to wrap what a shortcode
rendered.

That HTML is rendered, not sanitized. It comes from an installed plugin package, which
already executes in the server process — the same trust platzky extends to shortcode output
in post content, and filtering it would block nothing such a package could not do more
directly. The plugin's side of that bargain is to escape the *data* it interpolates.

.. _plugins-first-party-field-types:

Goodmap renders its own field types the same way. A value naming one of the types in
``goodmap/field_types.py`` — ``hyperlink`` and ``CTA`` — is rendered to ``html`` by goodmap
itself, so there are no built-in React field renderers left at all, and one URL policy
(platzky's, which admits ``http``, ``https``, ``mailto`` and ``tel``) rather than one in
Python and another in JavaScript. ``prepare_pin`` consults these only for a field no plugin
shortcode is bound to: a plugin still owns a field it is bound to by name, and a location
entry cannot name a type to redirect its own field at some other renderer.

A plugin cannot take over ``hyperlink`` or ``CTA`` either. The server always emits ``html``
for a type it renders, so that HTML is always the innermost stage — a field plugin attached
to one of these types wraps it and cannot replace it.

``hyperlink`` and ``CTA`` share a renderer, because they only ever differed in presentation:
both are a URL and the text to show for it. Which one a field is decides where the popup puts
it — a line among the details, or a button below them — which ``LocationDetails`` decides
from the field name.

A field plugin is a ``MarkerFieldPluginBase`` whose component is a stage
``({ input, config }) => element`` — it receives the previous stage's output as ``input``.
There's one kind of field plugin; what it does with ``input`` is what makes it read as a
Expand All @@ -134,7 +168,7 @@ rendering. Its platzky shortcode turns the raw value into ``{"type": "<field>",
}

**Wrap the input** — a later stage receives the current element and composes around it (e.g.
to customize a built-in ``hyperlink``/``CTA``). Needs no shortcode:
to customize a ``hyperlink`` or a ``CTA``). Needs no shortcode:

.. code-block:: jsx

Expand All @@ -150,7 +184,8 @@ to customize a built-in ``hyperlink``/``CTA``). Needs no shortcode:

Both are the same plugin kind. Each sets ``config.field`` to the type it attaches to and,
optionally, ``config.order``; lower order is more innermost, higher order wraps further out.
A wrapper must have a renderer beneath it (a built-in, or one it depends on).
A wrapper must have a renderer beneath it (a type rendered server-side — one of goodmap's
own, or a plugin's shortcode — or a renderer plugin it depends on).

.. _plugins-configuration:

Expand Down
12 changes: 7 additions & 5 deletions e2e-tests/tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def get_rightmost_marker(page: Page) -> ElementHandle | None:

def verify_popup_content(page: Page, expected_content: dict[str, Any]) -> None:
"""
Verifies popup content including title, subtitle, categories, and CTA button.
Verifies popup content including title, subtitle, categories, and CTA link.

Scopes assertions to .leaflet-popup-content or .MuiDialogContent-root
to avoid false positives from other elements on the page.
Expand Down Expand Up @@ -107,12 +107,14 @@ def verify_popup_content(page: Page, expected_content: dict[str, Any]) -> None:
# Check that the value appears at least once in the popup
expect(popup.get_by_text(value).first).to_be_visible()

# Verify and click CTA button if provided
# Verify the CTA if provided. It is a link the server rendered, styled as a button, so
# this checks where it points rather than clicking it — a target="_blank" click opens a
# second page and leaves the popup behind, which is not what the caller is asserting.
if "CTA" in expected_content:
cta = expected_content["CTA"]
cta_button = popup.locator("button", has_text=cta["displayValue"])
expect(cta_button).to_be_visible()
cta_button.click()
cta_link = popup.locator("a", has_text=cta["displayValue"])
expect(cta_link).to_be_visible()
expect(cta_link).to_have_attribute("href", cta["value"])


def verify_problem_form(page: Page) -> None:
Expand Down
19 changes: 12 additions & 7 deletions frontend/src/components/MarkerPopup/FieldRenderer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@ import React, { useReducer, useEffect } from 'react';
import PropTypes from 'prop-types';
import { getFieldPlugins, subscribe } from '../../plugins/pluginRegistry';
import getContentAsString from './fieldContent';
import { builtinFieldRenderers } from './builtinFieldRenderers';
import ServerHtmlField from './serverHtmlField';

/**
* Renders a marker field value as a pipe.
*
* The raw `value` flows through a chain of stages: the built-in for the field `type` (if
* any) renders it into an element, then each field plugin attached to that `type` (by
* The raw `value` flows through a chain of stages: the server-rendered `html` for the field
* (if there is any) seeds the fold, then each field plugin attached to that `type` (by
* `config.field`) transforms the result, innermost-first by `config.order`. Every stage is
* `({ input, config }) => element`, receiving the previous stage's output as `input` — so
* the innermost gets the raw value and renders from it, and each later stage gets the
* current element and wraps it.
*
* A wrapper therefore presupposes that something renders the type (a built-in, or a renderer
* plugin it ships with / depends on); a type with only wrappers and no renderer is a
* A wrapper therefore presupposes that something renders the type (the server's `html`, or a
* renderer plugin it ships with / depends on); a type with only wrappers and no renderer is a
* misconfiguration. With no stage at all, the value falls back to a string.
*
* Everything is computed during render, so a changed `value`/`type` is always reflected;
Expand All @@ -27,11 +27,16 @@ const FieldRenderer = ({ value }) => {
useEffect(() => subscribe(forceRender), []);

const type = value?.type;
const Builtin = type ? builtinFieldRenderers[type] : undefined;
const plugins = type ? getFieldPlugins(type) : [];

// The innermost stage renders the raw value: whatever rendered the field server-side
// seeds the fold with its own HTML, which is what lets both goodmap's first-party types
// and a platzky plugin display without shipping any frontend code. Wrappers wrap it, and
// because the seed is always there for a rendered type, they cannot replace it.
const Seed = value?.html ? ServerHtmlField : undefined;

const stages = [
...(Builtin ? [{ Stage: Builtin, config: undefined }] : []),
...(Seed ? [{ Stage: Seed, config: undefined }] : []),
...plugins.map(({ Plugin, config }) => ({ Stage: Plugin, config })),
];

Expand Down
25 changes: 25 additions & 0 deletions frontend/src/components/MarkerPopup/LocationDetails.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,33 @@ const DetailValue = styled.span`
word-break: break-word;
`;

// The CTA is a link the server rendered (goodmap/field_types.py), so its button look is
// styling rather than markup, and it belongs here — this container already selects exactly
// the CTA fields. An anchor rather than a button also gets middle-click, "copy link
// address" and a screen reader announcing where it goes, which the old onClick did not.
const CTAContainer = styled.div`
margin: 8px;

a {
display: flex;
align-items: center;
justify-content: center;
width: 100%;
height: 40px;
min-width: 50px;
padding: 0;
border: none;
border-radius: 8px;
background-color: ${() => globalThis.SECONDARY_COLOR || 'black'};
box-shadow: 0px 4px 6px rgba(0, 0, 0, 0.1);
color: white;
font-size: 18px;
line-height: 1;
text-align: center;
text-decoration: none;
cursor: pointer;
transition: background-color 0.2s ease-in-out;
}
`;

const ActionButton = styled.button`
Expand Down
78 changes: 0 additions & 78 deletions frontend/src/components/MarkerPopup/builtinFieldRenderers.jsx

This file was deleted.

30 changes: 30 additions & 0 deletions frontend/src/components/MarkerPopup/serverHtmlField.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import React from 'react';
import PropTypes from 'prop-types';

/**
* Renders the HTML the server produced for a field value.
*
* Two things arrive this way. A platzky plugin's shortcode renders its own field, and
* goodmap renders its own first-party types (`hyperlink`, `CTA` — see
* goodmap/field_types.py). Neither needs a component here, which is the point: a field
* type is added by teaching the server to render it, not by shipping React.
*
* The markup is not sanitized, and deliberately so. It is either goodmap's own or comes
* from an installed plugin package, which already runs arbitrary code in the server
* process — the same trust platzky extends to shortcode output in post content.
* Sanitizing would filter nothing such a package could not do more directly, while
* breaking legitimate markup. The obligation in return is to escape the *data* being
* interpolated, which is untrusted; `field_types.py` and `Shortcode.render` both do.
*
* This is the innermost stage of the fold, so a field plugin attached to the same `type`
* wraps it and cannot replace it. That is what stops a plugin taking over a first-party
* type: the server always emits `html` for one, so there is always a seed beneath the
* wrapper.
*/
const ServerHtmlField = ({ input }) => <span dangerouslySetInnerHTML={{ __html: input.html }} />;

Check warning on line 24 in frontend/src/components/MarkerPopup/serverHtmlField.jsx

View workflow job for this annotation

GitHub Actions / lint

Dangerous property 'dangerouslySetInnerHTML' found

ServerHtmlField.propTypes = {
input: PropTypes.shape({ html: PropTypes.string.isRequired }).isRequired,
};

export default ServerHtmlField;
9 changes: 0 additions & 9 deletions frontend/src/styles/buttonStyle.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,6 @@ const mapButton = {

export const buttonStyle = { ...mapButton };

export const MarkerCTAButtonStyle = {
...commonStyle,
fontSize: '18px',
width: '100%',
height: '40px',
minWidth: '50px',
borderRadius: '8px',
};

export const buttonStyleSmall = {
...commonStyle,
width: 'auto',
Expand Down
74 changes: 74 additions & 0 deletions frontend/tests/Map/Map.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import React from 'react';
import '@testing-library/jest-dom';
import { act } from '@testing-library/react';
import MapContainer from '../../src/components/Map/Map';

jest.mock('../../src/components/Map/MapComponent', () => () => <div data-testid="map-component" />);
jest.mock('../../src/components/FiltersForm/FiltersForm', () => () => (
<div data-testid="filters-form" />
));
jest.mock('../../src/components/common/AppToaster', () => () => null);
jest.mock('../../src/services/http/httpService', () => ({
__esModule: true,
default: {
getCategoriesData: jest.fn().mockResolvedValue({ categories: [], defaultChecked: {} }),
getLocationSchema: jest.fn().mockResolvedValue({}),
},
}));

// Map.jsx creates its own React root instead of being rendered by a test renderer,
// so act() has to be told this is an act environment.
globalThis.IS_REACT_ACT_ENVIRONMENT = true;

const renderApp = async () => {
await act(async () => {
MapContainer();
});
};

describe('MapWrap placeholders', () => {
let error;

// PropTypes' `node` validator does not recognise portals, so every render here
// warns about FiltersProvider's children (a pre-existing dev-only warning, not
// something these tests are about). console.error is silenced and asserted on by
// message instead of by call count.
beforeEach(() => {
error = jest.spyOn(console, 'error').mockImplementation(() => {});
});

afterEach(() => {
document.body.innerHTML = '';
jest.restoreAllMocks();
});

it('renders both portals when the left panel is present', async () => {
document.body.innerHTML = '<div id="map"></div><div id="filter-form"></div>';

await renderApp();

expect(document.querySelector('[data-testid="map-component"]')).not.toBeNull();
expect(document.querySelector('[data-testid="filters-form"]')).not.toBeNull();
});

// A deployment with no categories renders no left panel at all, so #filter-form is
// legitimately missing - the map must still come up rather than the whole app
// bailing out.
it('still renders the map when the filters placeholder is missing', async () => {
document.body.innerHTML = '<div id="map"></div>';
await renderApp();

expect(document.querySelector('[data-testid="map-component"]')).not.toBeNull();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(document.querySelector('[data-testid="filters-form"]')).toBeNull();
expect(error).not.toHaveBeenCalledWith(expect.stringContaining('render the map'));
});

it('renders nothing when the map placeholder is missing', async () => {
document.body.innerHTML = '<div id="filter-form"></div>';
await renderApp();

expect(document.querySelector('[data-testid="map-component"]')).toBeNull();
expect(document.querySelector('[data-testid="filters-form"]')).toBeNull();
expect(error).toHaveBeenCalledWith(expect.stringContaining('render the map'));
});
});
Loading
Loading