useWindowDimensions hook for Artboard viewport

[macintoshhelper]

Not sure if it's worth having context feature detection? Or just require people to run recent React versions, rather than have the hook act as a stub on earlier versions.

fixes #414

[mathieudutour]

That look amazing. Just a couple of comments

[mathieudutour]

So I'm not sure it's enough to only consider the viewport. Most of the time people won't specify it, and only use the style's width (like in the basic example https://github.com/airbnb/react-sketchapp/blob/master/examples/basic-setup/src/my-command.js#L45-L51).

So IMO it'd be useful to fallback to the style dimension if we have no viewport (even if we have only the width because it's probably the main use case).

[macintoshhelper]

Good point, am falling back to style.width and style.height now. Is it worth doing a console.warn in the hook, if the viewport is missing, prompting them to use the <Artboard viewport={} prop? Incase they get confused about why the height is the full page height rather than viewport height?

[mathieudutour]

Nah I think that what you actually expect: just the size of the artboard. I actually didn't even think about the viewport prop before your PR.

[mathieudutour]

Do you think it'd be useful to add scale and fontScale here? Even if it doesn't change the sketch output, it could be a nice way to provide a hook for people to change those values and control the context

[macintoshhelper]

Yes, would be great to pass a custom scale and fontScale to artboards, to better replicate React Native device-specific behaviour. Do you think this should be added to the viewport prop, rather than create a new prop? Not sure if it could create confusion if people think that the scale/fontScale is supposed to be handled by Sketch viewports?

[mathieudutour]

It feels natural in viewport and I guess with some docs it would be ok.

[macintoshhelper]

I've filtered out scale and fontScale from ArtboardRenderer.tsx so they don't pollute the generated file. Does this look good/readable?

react-sketchapp/src/renderers/ArtboardRenderer.ts

Lines 25 to 35 in aec1caa

	...(props.viewport && {
	presetDictionary: {
	allowResizedMatching: 0,
	offersLandscapeVariant: 1,
	...{
	name: props.viewport.name,
	width: props.viewport.width,
	height: props.viewport.height,
	},
	},
	}),

Updated the docs in API.md (note after the props table).

Would be really useful for testing/visualising conditional image quality based on device pixel density, etc.

[mathieudutour]

why not just

presetDictionary: { 
     allowResizedMatching: 0, 
     offersLandscapeVariant: 1, 

     name: props.viewport.name, 
     width: props.viewport.width, 
     height: props.viewport.height,      
},

?

The doc is good for me 👍

[mathieudutour]

it could be interesting to add 2 Artboard with different viewports and show what useWindowDimensions will be in both cases (as a comment below it) - and perhaps showing an additional div in case the width is bigger than a certain breakpoint

[macintoshhelper]

I've added a devices array and am mapping through that, and created a conditional View for tablet/desktop.

I've created a useBreakpoint hook that returns a getStyle function, like this: getStyle(['column', 'row']) – for column on mobile and row on tablet/desktop. Might be worth featuring in a new doc file, probably overkill for API.md example though.

[mathieudutour]

That's perfect, well done 👏

[mathieudutour]

I'm going to publish it, feel free to tweet about it if you want - it's a really cool feature and could make for a really visual tweet