Prototype flows with HotSpot and Artboard ids

[macintoshhelper]

• New <HotSpot> component
• New id and isHome prop for <Artboard> to use for prototypes

Includes tests and documentation.

ref #280

[mathieudutour]

I'm wondering if we need to introduce a new id prop 🤔

I think just having the name and making it the "hardcoded" seed all the time for artboard should be good enough. Maybe we can prefix the name with artboard: to make it won't conflict with other layers.

What do you think?

[macintoshhelper]

I implemented the new id prop incase someone has duplicate artboard names, or leaves the default name of Artboard. I was trying to avoid cases of duplicate UUIDs, but maybe in that case, there should be a console warning instead? The id prop should match the flow.target input also, so a layer display name might not necessarily be what someone would want to use in flow={{ target: 'id-name' }}.

Maybe I could change it to name though, and leave it as a random UUID if name is left as Artboard, and leave the id prop to override the name if someone wants?

[macintoshhelper]

Thoughts on the solution below? I replace hardcoded with an index prop to retrieve an ID by index, this means if there's duplicate artboards with the same name, the prototype link will go to the first one.

I think it would be worth using name, but with id as an optional override if you want a different layer display name.

generateID function:

export function generateID(seed?: ?string, index?: number): string {
  let _seed: ?string;

  if (seed && !index) {
    if (!previousSeeds[seed]) {
      previousSeeds[seed] = 0;
    }
    previousSeeds[seed] += 1;

    _seed = `${seed}${previousSeeds[seed]}`;
  } else if (seed && index) {
    _seed = `${seed}${index}`;
  }

  return e7(_seed);
}

ArtboardRenderer.js

...
do_objectID: generateID(props.id || props.name, 1),
...

[mathieudutour]

I was more thinking something like:

export function generateID(seed?: ?string, hardcoded?: boolean): string {
  let _seed: ?string;

  if (seed) {
    if (!previousSeeds[seed]) {
      previousSeeds[seed] = 0;
    }
    previousSeeds[seed] += 1;

    if (!hardcoded) {
      _seed = `${seed}${previousSeeds[seed]}`;
    }
  }

  return e7(_seed);
}

and in ArtboardRenderer.js

...
do_objectID: generateID(`artboard:${props.name}`, true),
...

(so that we are sure it doesn't conflict with other types of layers

[macintoshhelper]

I prefer having an id optional override, so a dev can use flow={{ target: '/about:mobile' }} and a designer/Sketch user would see About | Mobile in the layers, and using index instead of hardcoded should work nicer for duplicate artboard names/ids I think?

I can revert to your proposed generateID and remove the id prop if you'd prefer though. Good point on the artboard: prefix 👍

Updated code diff:
6d0d887

[mathieudutour]

so a dev can use flow={{ target: '/about:mobile' }} and a designer/Sketch user would see About | Mobile in the layers

but why would the dev not do flow={{ target: 'About | Mobile' }}? It's really Sketch specific anyway so I don't think it matters if it looks like a URL or not, right?

using index instead of hardcoded should work nicer for duplicate artboard names/ids

But using an index hardcoded as 1 instead of just a boolean won't change anything and might actually conflict with an existing previousSeeds 🤔

[mathieudutour]

I think I'd rather not add a new prop for now so that we don't expand the API surface (a lot harder to remove stuff than adding it), but let's keep it in mind if it turns out to be a pain

[macintoshhelper]

👍Applied your suggestions. I agree that accepting an id prop for <Artboard> is messy and could lead people to expect the same prop to work for other components (inconsistency).

Would you have any thoughts on exposing internal API access for things like a seed override to library developers (on the lines of dangerouslySetInnerHTML)?

For react-sketchapp-router, I'd like to be able to do <Link to="/about"> for an artboard with name "About | Mobile". Although, I can achieve that with an abstraction of an internal map of paths and artboard names in the <Link> and <Route> components, Link would have some magic anyway, so maybe that would be fine.

[macintoshhelper]

I've reverted to hardcoded and used your code for generateID, but in my testing, I found that the function with index handles duplicate artboard names gracefully by defaulting to the first artboard in the tree, where with hardcoded, duplicate artboard names results in duplicate object IDs and the artboard breaks in the prototype.

I just noticed I had a typo in my index code though and left in a stray index in the setter version, I meant to have this:

...
do_objectID: generateID(props.id || props.name),
...

(with return generateID(`artboard:${target}`, 1); in getArtboard)

That should solve the issue of existing previousSeeds conflicts I think? So the "setter" version of the function is used when rendering the artboard, without index, and the "getter" version of the function is where you pass the index argument of 1 (without incrementing). Hope that makes sense 🙂

[mathieudutour]

so I see that for a HotSpot layer, you generate a normal View with the flow property.

Sketch has a notion of HotSpot which is separated from a normal layer. I understand why you'd want to generate a normal layer but then why do we need another layer type? Could we just add another prop to View?

[macintoshhelper]

True, yes. I actually ran into an issue where I was using my UI lib which wraps react-primitives and styled-system to turn <View> into <Box>, etc, and found myself using <Box injectedProps={{ flow: {...} }} which is messy. I'll move the flow prop to <View>.

Should src/index.js have a HotSpot export which is a reference to <View>? e.g.

import View from './components/View';

const HotSpot = View;

module.exports = {
  ...
  View,
  HotSpot,
  ...

Might be messy though if people are importing HotSpot and it is removed in the future.

[mathieudutour]

let's forget about HotSpot for now then

[mathieudutour]

👍

[macintoshhelper]

👍

Thanks! I'll start work on an initial release of react-sketchapp-router 🙂