Skip to content

Commit

Permalink
Fix content path links in usage view to scroll to the correct element
Browse files Browse the repository at this point in the history
  • Loading branch information
laymonage authored and thibaudcolas committed Sep 24, 2024
1 parent 8661056 commit 3b22cbf
Show file tree
Hide file tree
Showing 9 changed files with 210 additions and 5 deletions.
7 changes: 6 additions & 1 deletion client/src/includes/panels.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getElementByContentPath } from '../utils/contentPath';

/**
* Switches a collapsible panel from expanded to collapsed, or vice versa.
* Updates the DOM and fires custom events for other code to hook into.
Expand Down Expand Up @@ -110,7 +112,10 @@ export function initCollapsiblePanels(
export function initAnchoredPanels(
anchorTarget = document.getElementById(window.location.hash.slice(1)),
) {
const target = anchorTarget?.matches('[data-panel]') ? anchorTarget : null;
const target = anchorTarget?.matches('[data-panel]')
? anchorTarget
: getElementByContentPath();

if (target) {
setTimeout(() => {
target.scrollIntoView({ behavior: 'smooth' });
Expand Down
6 changes: 4 additions & 2 deletions client/src/includes/tabs.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getElementByContentPath } from '../utils/contentPath';
/**
* All tabs and tab content must be nested in an element with the data-tab attribute
* All tab buttons need the role="tab" attr and an href with the tab content ID
Expand Down Expand Up @@ -271,9 +272,10 @@ class Tabs {
selectTabByURLHash() {
if (window.location.hash) {
const anchorId = window.location.hash.slice(1);
const anchoredElement =
document.getElementById(anchorId) || getElementByContentPath();
// Support linking straight to a tab, or to an element within a tab.
const tabID = document
.getElementById(anchorId)
const tabID = anchoredElement
?.closest('[role="tabpanel"]')
?.getAttribute('aria-labelledby');
const tab = document.getElementById(tabID);
Expand Down
17 changes: 17 additions & 0 deletions client/src/includes/tabs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,23 @@ describe('tabs', () => {
expect(promoteTabLabel.getAttribute('aria-selected')).toEqual('true');
});

it('should select the correct tab where the element pointed by the contentpath directive lives', async () => {
window.location.hash = '#:w:contentpath=search_description';
initTabs();
await Promise.resolve();

const contentTab = document.getElementById('tab-content');
const promoteTab = document.getElementById('tab-promote');
const contentTabLabel = document.getElementById('tab-label-content');
const promoteTabLabel = document.getElementById('tab-label-promote');

expect(contentTab.hasAttribute('hidden')).toBe(true);
expect(promoteTab.hasAttribute('hidden')).toBe(false);

expect(contentTabLabel.getAttribute('aria-selected')).toEqual('false');
expect(promoteTabLabel.getAttribute('aria-selected')).toEqual('true');
});

it('should not throw an error if the URL hash begins with a number', async () => {
window.location.hash = '#123abcd';
initTabs();
Expand Down
91 changes: 91 additions & 0 deletions client/src/utils/contentPath.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import {
getWagtailDirectives,
getContentPathSelector,
getElementByContentPath,
} from './contentPath';

describe('getWagtailDirectives', () => {
afterEach(() => {
window.location.hash = '';
});

it('should return the directive after the delimiter as-is', () => {
window.location.hash = '#:w:contentpath=abc1.d2e.3f';
expect(getWagtailDirectives()).toEqual('contentpath=abc1.d2e.3f');
});

it('should allow a normal anchor in front of the delimiter', () => {
window.location.hash = '#an-anchor:w:contentpath=abc1.d2e.3f';
expect(getWagtailDirectives()).toEqual('contentpath=abc1.d2e.3f');
});

it('should allow multiple values for the same directive', () => {
window.location.hash =
'#hello:w:contentpath=abc1.d2e.3f&unknown=123&unknown=456';
expect(getWagtailDirectives()).toEqual(
'contentpath=abc1.d2e.3f&unknown=123&unknown=456',
);
});
});

describe('getContentPathSelector', () => {
it('should return a selector string for a single content path', () => {
expect(getContentPathSelector('abc1')).toEqual('[data-contentpath="abc1"]');
});
it('should allow dotted content path', () => {
expect(getContentPathSelector('abc1.d2e.3f')).toEqual(
'[data-contentpath="abc1"] [data-contentpath="d2e"] [data-contentpath="3f"]',
);
});

it('should ignore leading, trailing, and extra dots', () => {
expect(getContentPathSelector('.abc1...d2e..3f.')).toEqual(
'[data-contentpath="abc1"] [data-contentpath="d2e"] [data-contentpath="3f"]',
);
});

it('should return an empty string if content path is an empty string', () => {
expect(getContentPathSelector('')).toEqual('');
});
});

describe('getElementByContentPath', () => {
beforeEach(() => {
document.body.innerHTML = /* html */ `
<div id="one" data-contentpath="abc1">
<div id="two" data-contentpath="d2e">
<div id="three" data-contentpath="3f"></div>
</div>
<div id="four" data-contentpath="g4h"></div>
</div>
`;
});

afterEach(() => {
window.location.hash = '';
});

it('should return the element for a single content path', () => {
const element = getElementByContentPath('abc1');
expect(element).toBeTruthy();
expect(element.id).toEqual('one');
});

it('should return the element for a dotted content path', () => {
const element = getElementByContentPath('abc1.d2e.3f');
expect(element).toBeTruthy();
expect(element.id).toEqual('three');
});

it('should read from the contentpath directive if there is one', () => {
window.location.hash = '#:w:contentpath=abc1.d2e.3f';
const element = getElementByContentPath();
expect(element).toBeTruthy();
expect(element.id).toEqual('three');
});

it('should return null if it cannot find the element', () => {
expect(getElementByContentPath('abc1.d2e.3f.g4h')).toBeNull();
expect(getElementByContentPath()).toBeNull();
});
});
78 changes: 78 additions & 0 deletions client/src/utils/contentPath.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
const WAGTAIL_DIRECTIVE_DELIMITER = ':w:';

/**
* Extract the Wagtail directives from the URL fragment.
*
* This follows the algorithm described in
* https://wicg.github.io/scroll-to-text-fragment/#extracting-the-fragment-directive
* for extracting the fragment directive from the URL fragment, with a few
* differences:
* - We use a :w: delimiter instead of the proposed :~: delimiter.
* - We don't remove our directive from the URL fragment.
*
* @param rawFragment The raw fragment (hash) from the URL,
* @returns a string of Wagtail directives, if any, in the style of URL search parameters.
*
* @example window.location.hash = '#:w:contentpath=abc1.d2e.3f'
* // getWagtailDirectives() === 'contentpath=abc1.d2e.3f'
*
* @example window.location.hash = '#an-anchor:w:contentpath=abc1.d2e.3f'
* // getWagtailDirectives() === 'contentpath=abc1.d2e.3f'
*
* @example window.location.hash = '#hello:w:contentpath=abc1.d2e.3f&unknown=123&unknown=456'
* // getWagtailDirectives() === 'contentpath=abc1.d2e.3f&unknown=123&unknown=456'
*/
export function getWagtailDirectives() {
const rawFragment = window.location.hash;
const position = rawFragment.indexOf(WAGTAIL_DIRECTIVE_DELIMITER);
if (position === -1) return '';
return rawFragment.slice(position + WAGTAIL_DIRECTIVE_DELIMITER.length);
}

/**
* Compose a selector string to find the content element based on the dotted
* content path.
*
* @param contentPath dotted path to the content element.
* @returns a selector string to find the content element.
*
* @example getContentPathSelector('abc1.d2e.3f')
* // returns '[data-contentpath="abc1"] [data-contentpath="d2e"] [data-contentpath="3f"]'
*/
export function getContentPathSelector(contentPath: string) {
const pathSegments = contentPath.split('.');
const selector = pathSegments.reduce((acc, segment) => {
// In some cases the segment can be empty, e.g. when the path ends with
// a trailing dot, which may be the case with inline panels.
if (!segment) return acc;

const segmentSelector = `[data-contentpath="${segment}"]`;
return acc ? `${acc} ${segmentSelector}` : segmentSelector;
}, '');
return selector;
}

/**
* Get the content element based on a given content path (or one extracted from
* the URL hash fragment).
*
* @param contentPath (optional) content path to the content element. If not
* provided, it will be extracted from the URL fragment.
* @returns the content element, if found, otherwise `null`.
*
* @example getElementByContentPath('abc1.d2e.3f')
* // returns <div data-contentpath="3f">...</div>
*
* @example getElementByContentPath()
* // with an URL e.g. https://example.com/#:w:contentpath=abc1.d2e.3f
* // returns <div data-contentpath="3f">...</div>
*/
export function getElementByContentPath(contentPath?: string) {
const path =
contentPath ||
new URLSearchParams(getWagtailDirectives()).get('contentpath');

return path
? document.querySelector<HTMLElement>(getContentPathSelector(path))
: null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{% for reference in value %}
<li>
{% if edit_url %}
<a href="{{ edit_url }}#content-path-{{ reference.content_path }}">
<a href="{{ edit_url }}#:w:contentpath={{ reference.content_path }}">
{% endif %}
{{ reference.describe_source_field }}{% if edit_url %}</a>{% endif %}{% if describe_on_delete %}: {{ reference.describe_on_delete }}{% endif %}
</li>
Expand Down
4 changes: 3 additions & 1 deletion wagtail/admin/tests/pages/test_page_usage.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ def test_has_editable_usage(self):

self.assertContains(response, "Contact us")
self.assertContains(
response, reverse("wagtailadmin_pages:edit", args=(form_page.id,))
response,
reverse("wagtailadmin_pages:edit", args=(form_page.id,))
+ "#:w:contentpath=thank_you_redirect_page",
)
self.assertContains(response, "Thank you redirect page")
self.assertContains(response, "<td>Form page with redirect</td>", html=True)
Expand Down
5 changes: 5 additions & 0 deletions wagtail/admin/tests/viewsets/test_model_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,11 @@ def test_simple(self):
link = tds[0].select_one("a")
self.assertIsNotNone(link)
self.assertEqual(link.attrs.get("href"), tbx_edit_url)
content_path_link = tds[-1].select_one("a")
self.assertEqual(
content_path_link.attrs.get("href"),
tbx_edit_url + "#:w:contentpath=cascading_toy",
)

# Link to referrer's edit view with parameters for the specific field
link = tds[2].select_one("a")
Expand Down
5 changes: 5 additions & 0 deletions wagtail/snippets/tests/test_usage.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ def test_usage(self):
self.assertContains(response, "<th>Field</th>", html=True)
self.assertNotContains(response, "<th>If you confirm deletion</th>", html=True)
self.assertContains(response, "Snippet content object")
self.assertContains(
response,
reverse("wagtailadmin_pages:edit", args=[gfk_page.id])
+ "#:w:contentpath=snippet_content_object",
)

def test_usage_without_edit_permission_on_snippet(self):
# Create a user with basic admin backend access
Expand Down

0 comments on commit 3b22cbf

Please sign in to comment.