Bug 1891121 - Add experiment brief links to experiment and rollout tables #524
Conversation
✅ Deploy Preview for fxms-skylight ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
|
We need to request that the experimenter team do it (or put in a PR ourselves and ask for their review). |
sarahhjchung
force-pushed
the
bug-1891121
branch
from
19d3652 to
deed77f
Compare
There was a problem hiding this comment.
This is looking good! A couple of UX things that I think we want to tweak though:
- In all of them, I think the experiment slug wants to be indented to start in the same place as the human-readable experiment name, as it's visually a bit confusing as-is. That said, that would line the experiment info up with the branch info, so maybe we'd want to indent the branches more too. Play around a bit and see what looks reasonable.
- In the Mozilla VPN one, if it's easy to prevent wrapping (eg with
) a line-break just before the symbol, I think the visual presentation would be smoother.
app/columns.tsx
Outdated
| <svg | ||
| fill="currentColor" | ||
| fillOpacity="0.6" | ||
| viewBox="0 0 8 8" | ||
| className="inline h-[1.2rem] w-[1.2rem] px-1" | ||
| aria-hidden="true" | ||
| style={{ | ||
| marginInline: "0.1rem 0", | ||
| paddingBlock: "0 0.1rem", | ||
| overflow: "visible", | ||
| }} | ||
| > | ||
| <path d="m1.71278 0h.57093c.31531 0 .57092.255837.57092.571429 0 .315591-.25561.571431-.57092.571431h-.57093c-.31531 0-.57093.25583-.57093.57143v4.57142c0 .3156.25562.57143.57093.57143h4.56741c.31531 0 .57093-.25583.57093-.57143v-.57142c0-.3156.25561-.57143.57092-.57143.31532 0 .57093.25583.57093.57143v.57142c0 .94678-.76684 1.71429-1.71278 1.71429h-4.56741c-.945943 0-1.71278-.76751-1.71278-1.71429v-4.57142c0-.946778.766837-1.71429 1.71278-1.71429zm5.71629 0c.23083.0002686.43879.13963.52697.353143.02881.069172.04375.143342.04396.218286v2.857141c0 .31559-.25561.57143-.57093.57143-.31531 0-.57092-.25584-.57092-.57143v-1.47771l-1.88006 1.88171c-.14335.14855-.35562.20812-.55523.15583-.19962-.0523-.35551-.20832-.40775-.40811-.05225-.19979.00727-.41225.15569-.55572l1.88006-1.88171h-1.47642c-.31531 0-.57093-.25584-.57093-.571431 0-.315592.25562-.571429.57093-.571429z"></path> |
There was a problem hiding this comment.
Can you add an XXX comment both here and later in the file that we should switch to the Lucide Icon for an external link instead of maintaining our own? (I don't think we need to fix this now).
package.json
Outdated
| @@ -19,6 +19,7 @@ | |||
| "dependencies": { | |||
| "@auth0/nextjs-auth0": "^3.6.0", | |||
| "@looker/sdk-node": "25.4.0", | |||
| "@mozilla/nimbus-schemas": "^3001.0.0", | |||
| "@mozilla/nimbus-shared": "^2.5.2", | |||
There was a problem hiding this comment.
From reading https://github.com/mozilla/nimbus-shared, it sounds like we don't need nimbus-shared any more.
| const nimbusExperimentV7Schema = require("@mozilla/nimbus-schemas/schemas/NimbusExperimentV7.schema.json"); | ||
| type NimbusExperiment = typeof nimbusExperimentV7Schema.properties; | ||
| type DocumentationLink = | ||
| typeof nimbusExperimentV7Schema.properties.documentationLinks; |
There was a problem hiding this comment.
I think it would be helpful to put JSDoc comments here explaining why you're doing the type aliasing.
|
I played around with the spacing and indenting and concluded on this. 
I've made the user facing name experiment/rollout (ie. the bold text) to have the |
Bug 1891121
Changes made:
RecipeInfonow includes anexperimentBriefLinkproperty with the url to experiment link Google docsFileTexticon from lucide-icons is now displayed next to the experiment/rollout name if an experiment brief link exists@mozilla/nimbus-schemashas been imported and replaces theNimbusExperimenttype used from@mozilla/nimbus-shared