Add article that renders IDP analytics events from JSON (LG-5590)#218
Add article that renders IDP analytics events from JSON (LG-5590)#218zachmargolis merged 10 commits intomainfrom
Conversation
zachmargolis
requested review from
aduth,
gsa-manish,
mitchellhenke,
stevegsa and
theabrad
- Federalist no longer supports overriding _config.yml
| import { h, Component, render } from '../../preact.module.js'; | ||
| import htm from '../../htm.module.js'; | ||
|
|
||
| // Copied from https://github.com/bryanbraun/anchorjs |
There was a problem hiding this comment.
While unfortunate 'til more packages ship ESM-native code, this seems okay enough.
A few alternatives that pop to mind, none of which I'm particularly compelled toward:
- Import from an ESM-upgrading CDN like Skypack or unpkg
?module - Use something like Snowpack
- I was more fond of the v1 behavior, which was mostly minimalistic as a way to create local, ESM-upgraded versions of CJS packages. It's now pretty feature-rich, more than we might need or want.
- Just use a bundler 🤷 ESBuild could work well, since we don't need much transpilation beyond module bundling, and could let us use JSX/TypeScript.
| const setRef = (dom) => { | ||
| if (dom && document.location.hash.slice(1) === slug) { | ||
| setTimeout(() => dom.scrollIntoView(), 0); |
There was a problem hiding this comment.
Minor: The name dom suggests to me that this is a full DOM object (e.g. a JSOM DOM wrapper object), when in-fact it's just an element node. I might have used element or node instead.
|
|
||
| function Example({ event_name, attributes }) { | ||
| function typeExample(type) { | ||
| switch(type) { |
There was a problem hiding this comment.
For future: Would be nice to set up ESLint/Prettier in this project.
|
|
||
| const eventProperties = attributeExamples. | ||
| join("\n "). | ||
| replace(/( $)/, ''); // fix last line indentation |
There was a problem hiding this comment.
Nit/Question: Group shouldn't be necessary?
| replace(/( $)/, ''); // fix last line indentation | |
| replace(/ $/, ''); // fix last line indentation |
There was a problem hiding this comment.
Would String#trimEnd have worked here?
| replace(/( $)/, ''); // fix last line indentation | |
| trimEnd(); // fix last line indentation |
There was a problem hiding this comment.
Parens were optional, but turns out trimEnd() removes a trailing newline that I wanted to keep for bracket alignment.
There's probably a better way to manage building these fixed-width examples than futzing with indentation and newlines like this
| return html`${events.map((event) => | ||
| html`<${Event} ...${event} />` | ||
| )}`; |
There was a problem hiding this comment.
Outer html might not be necessary?
| return html`${events.map((event) => | |
| html`<${Event} ...${event} />` | |
| )}`; | |
| return events.map((event) => html`<${Event} ...${event} />`); |
There was a problem hiding this comment.
Yup! this worked
| } | ||
|
|
||
| function Sidenav({ events }) { | ||
| return html`${events.map(({ event_name: name }) => |
There was a problem hiding this comment.
Same note about outer html, unless I'm mistaken (haven't tested).
| const { idpBaseUrl } = container.dataset; | ||
| const eventsUrl = `${idpBaseUrl}/api/analytics-events`; | ||
|
|
||
| const sidenav = document.querySelector('#sidenav'); |
There was a problem hiding this comment.
Not impactful here, but micro-optimization note that getElementById is ~2x faster than querySelector.
https://www.measurethat.net/Benchmarks/Show/2488/0/getelementbyid-vs-queryselector
| const sidenav = document.querySelector('#sidenav'); | |
| const sidenav = document.getElementById('sidenav'); |
3 participants
See 18F/identity-idp#6014
This is basically the same idea as GSA-TTS/identity-site#707, pulling JSON and making a page from it. This time I figured we don't care as much about browser compatibility, since it's our handbook, that I didn't bother with that and just went with inline ES6. The future is now!
Screenshot of what it looks like with the IDP serving this data