Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Update APIRef macro for new event organisation #1313

Closed
wbamberg opened this issue Apr 11, 2019 · 11 comments
Closed

Update APIRef macro for new event organisation #1313

wbamberg opened this issue Apr 11, 2019 · 11 comments
Assignees

Comments

@wbamberg
Copy link

wbamberg commented Apr 11, 2019

This is a work item for #685, and was originally discussed in #906.

The APIRef macro currently refers to GroupData to list events. We need to change this as part of the event refactoring wok, and instead treat events more like other subpages of the API (e.g. properties and methods).

But APIRef is complex, used in many pages, and has no tests. So a prerequisite for changing it is adding tests.

In this user story we will:

AC are that both those changes are committed to KumaScript, deployed, and verified.

@wbamberg
Copy link
Author

wbamberg commented Apr 19, 2019

@Elchi3 , @chrisdavidmills , I've run into a problem with this and I'd like to know what you think we should do.

I've written tests for APIRef's current behavior, and that's merged. I then updated the macro to look in subpages for events, as it does for properties and methods. This is easy, but: the macro uses the page titles for the link text, which, given our format of "Interface: eventname event" gives us a sidebar like:

Screen Shot 2019-04-18 at 7 26 35 PM

...which looks terrible.

The easy hacky thing to do is munge the titles in the macro, to try to transform, say, SpeechRecognition: audioend event into audioend. I don't really like this because I think it's likely to be fragile, but I think this is probably what I'll end up doing. Do you have any better ideas?

@ExE-Boss
Copy link

ExE-Boss commented Apr 19, 2019

You could do the following, which is more robust.

/**
 * @param {string} title
 * @param {string} interfaceName
 */
function processEventTitle(title, interfaceName) {
	let trimStart = 0;
	let trimEnd = title.length;
	if (title.startsWith(interfaceName + ':')) {
		trimStart = interfaceName.length + 1;
	}
	if (title.endsWith(' event') {
		trimEnd -= 6; // ' event'.length
	}
	return title.substring(trimStart, trimEnd).trim();
}

@wbamberg
Copy link
Author

wbamberg commented Apr 19, 2019

@ExE-Boss , that's what I meant by "munge the titles in the macro". Of course I understand that it's possible to write more or less complicated string-munging code here. But my point is that doing a transformation like this is inherently fragile. For example, how will this work for Arabic or Chinese titles? More generally, though, how are we to guarantee that titles follow the format that our hacky code assumes? And how are pages authors to know that somewhere, some hacky code is relying on page titles having a given format?

It feels like a more correct approach is to model this, so a page can indicate (in its metadata, say)

this is the name I want to use when I'm being included in a place where my context is clear

...but this seems like a much bigger change.

Also this problem is not specific to event pages. It's just never come up before because we've never used different titles before. But now we are experimenting with different titles, it's coming up there too: see for example the first item under "Properties" in the sidebar at https://developer.mozilla.org/en-US/docs/Web/CSS/border-image.

(I think this doesn't happen for HTML pages because they use the slug, not the title. I guess we could do that, but it's still fragile.)

@ExE-Boss
Copy link

ExE-Boss commented Apr 19, 2019

We could use the slug everywhere.

I run into that title issue with :where() at one point.

See also :has(), which is running into this at this very moment.

@chrisdavidmills
Copy link
Contributor

This is an interesting and tricky problem

I think the main thing causing it to look messy is that it goes onto multiple lines.

To mitigate this for the events in particular, could we just remove ParentInterface + separator (": " in the case of English) and leave "event" (or whatever the equivalent is in other languages)? Only having to remove one side would probably be less brittle/difficult than having to trim both sides?

You could do some research into how other languages would show the interface and separator, but I think this might make it easier.

In terms of overly long page titles in general, could we mitigate the problem by using https://developer.mozilla.org/en-US/docs/Web/CSS/text-overflow; text-overflow: ellipsis perhaps?

@ExE-Boss
Copy link

The multi‑line case used to be so much worse before mdn/kuma#5131 (review)

@wbamberg
Copy link
Author

I think the main thing causing it to look messy is that it goes onto multiple lines.

To mitigate this for the events in particular, could we just remove ParentInterface + separator (": " in the case of English) and leave "event" (or whatever the equivalent is in other languages)?

That's probably a bit less fragile, and it does look better, but tbh it still looks pretty silly:

Screen Shot 2019-04-23 at 7 42 19 AM

I mean: we know that they are events, since they're under the "Events" subheading. So why does it have to tell us again and again? Also it makes the list harder to scan. cf:

Screen Shot 2019-04-23 at 8 04 33 AM

I think: any mechanism that's based on munging strings (from title or slug) is inherently fragile, because it relies on an implicit contract with the title/slug to have a particular structure. We have no mechanism to enforce this structure or to flag cases (by linting, for example) where the structure isn't what we expect. We also have no explicit representation of this assumption (meaning that, for example, someone editing a page title would have no way of knowing that particular changes to it would break sidebars).

But: if we are to go with one of these options, munging the slug (just splitting on "_" and using the first part) is probably the least fragile and would have the best effect.

Unfortunately other options (like explicitly modelling this in the page metadata) seem like much more expensive things to do.

It's sad that the corner we've painted ourselves into with Kuma is such that these kinds of hacks seem like the only option, and that there are already so many hacks like this that it's easy to think "we'll just add another".

@chrisdavidmills
Copy link
Contributor

I can't say I disagree with any of this ;-)

I see you've put the PR in already; it looks to me to be the least worst option.

@wbamberg
Copy link
Author

I see you've put the PR in already; it looks to me to be the least worst option.

Yes, I thought it would be better to have something concrete to look at :)

@wbamberg
Copy link
Author

This change is merged, deployed and the new sidebar is visible in pages like https://developer.mozilla.org/en-US/docs/Web/API/SpeechRecognition/audiostart_event and https://developer.mozilla.org/en-US/docs/Web/API/IDBOpenDBRequest.

@chrisdavidmills
Copy link
Contributor

@wbamberg this is great work Will, thanks for driving this forward.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
@chrisdavidmills @wbamberg @ExE-Boss and others