Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Responsive Cosmos #539

Merged
merged 31 commits into from Jul 7, 2018
Merged

Responsive Cosmos #539

merged 31 commits into from Jul 7, 2018

Conversation

maxsalven
Copy link
Collaborator

@maxsalven maxsalven commented Dec 2, 2017

As far as I can see, Cosmos really struggles to be useful when working with responsive layouts / media queries. In Chrome, the only way I can get a media query to trigger is to go full screen and make my window small. Using the device toolbar in Dev Tools doesn't work, even in full screen mode.

Does anyone have any better 'no work required' solutions? I'd love to delete this PR!

I've whipped up a very quick proof of concept PR for adding responsive development tools to Cosmos:

responsivecosmos

The UX is very much a placeholder, but the rough idea is that users can define their own list of dimensions and labels (perhaps via their cosmos.config), which we show on the top. I'll also add support for an adhoc dimension option directly in the UX. You can trigger the responsive layout directly from a fixture with a specific dimension, or you can toggle it with the new responsive button next to the full screen button. I'd also like to add support for setting the background color, but perhaps that's better done in a proxy?

If a dimension is too big to fit in your window, we faithfully reproduce it and scale the contents, so you can still see exactly what your content might look like on e.g. a 1080p screen.

Credits due to https://docs.catalog.style/specimens/react#display-all-defined-screen-sizes-1 where I got some of the ideas (and CSS!) for this.

This originally started off as a proxy, but a brief chat with @skidding a couple weeks ago suggested this will be better both in terms of the development side and the user experience side if it's a core feature.

However, I need some help from @skidding please. If you load a fixture that has props, and change the screen size, it crashes. Hopefully it's something obvious, any thoughts?

I didn't want to put too much effort in yet until there's no 'major' blockers in terms of getting the cosmos fundamentals to work with this model.

@xavxyz
Copy link
Collaborator

xavxyz commented Dec 3, 2017

👋 hey!

That is awesome!! 🚀

Maybe we could integrate with Sizzy?

A tool for developing responsive websites crazy-fast

The code is open-source (repo) and it started as a Storybook plugin (story): there may still be a way to extract the core component and re-use it in the Loader!

@maxsalven
Copy link
Collaborator Author

@xavcz looks cool thanks, definitely some great ideas in there even if we don't want to directly adapt their product.

I'd like to circle back to a UX/UI discussion once @skidding confirms that this proof of concept PR is 'workable' (/fixable!). Would also be great to know how hard it would be to have Cosmos render to multiple iFrames at once for that Sizzy style 'multiple devices at once' layout.

@ovidiuch
Copy link
Member

ovidiuch commented Dec 3, 2017

This is great! 😍

I'll only answer the burning questions for now and leave the UX for later.

However, I need some help from @skidding please. If you load a fixture that has props, and change the screen size, it crashes. Hopefully it's something obvious, any thoughts?

The fixtureEdit event expects the entire (serializable parts of) the fixture :). More on the serializable aspect some other time, but this small change should unblock you for now:

screenshot 2017-12-03 17 42 57

Would also be great to know how hard it would be to have Cosmos render to multiple iFrames at once for that Sizzy style 'multiple devices at once' layout.

Yes. It ties into #383 and we definitely want to start rendering multiple Loader iframes at once. I've been talking with @xavcz about this and about the idea of porting the window.postMessage communication to a websockets connection via the server. But thinking of it I realize that while doing that sounds very cool we shouldn't need to do it to accomplish any of these goals. The websockets communication becomes truly relevant once we start thinking about React Native. So back to this. Yes, we should be able to render multiple iframes, but it's likely we'll need to convert the single channel communication between Playground and Loader to multi channel, by introducing a loaderId in each message. The way I see it, we can have the Loader pick up its ID from iframe url (eg. ?loaderId=5), and then use it in every message it sends to the parent. The Playground should also do some minimal state management to keep tabs of all its active loaders after assigning unique IDs to each frame.

@maxsalven
Copy link
Collaborator Author

maxsalven commented Dec 4, 2017

Thanks @skidding.

Is there currently active work on the multiple iFrame system? On a related note, I think as part of the 3.0 release you wanted to write some sort of roadmap?

Other quick question; in the current master, the ComponentPlayground fixtures show a "Cannot GET /mock/loader/index.html", is this intentional?
screen shot 2017-12-04 at 11 13 41 am

I'll circle back with a UX discussion for this PR later.

@ovidiuch ovidiuch mentioned this pull request Dec 9, 2017
@ovidiuch
Copy link
Member

ovidiuch commented Dec 9, 2017

Is there currently active work on the multiple iFrame system? On a related note, I think as part of the 3.0 release you wanted to write some sort of roadmap?

Aaaand.. it's here #546

@ovidiuch
Copy link
Member

ovidiuch commented Dec 9, 2017

Other quick question; in the current master, the ComponentPlayground fixtures show a "Cannot GET /mock/loader/index.html", is this intentional?

Forgot to answer this. Yes. We might be able to use the real Loader path in the loaded Playground, but then we'd get an infinite loop of Playground in Playground in Playground.

Which would actually pretty cool, but I remembered why it isn't possible at the moment. Because Loader communication is global, the Playground in Playground in Playground would send events to the root level Playground and confuse it. Once we have loaderId on Loader messages this will become possible.

@maxsalven
Copy link
Collaborator Author

maxsalven commented Dec 17, 2017

Hi all, I've added a few updates to this. Feel free to pull the branch and fire up yarn start:playground.

From a personal perspective with regards to my available time to work on this, I'd like to publish small, iterative updates to this feature, rather than try to do everything possible in one massive PR.

What I've pushed so far is a basic version of "level 1". No need for code review or UI review at this stage, please let's look at the API and the UX. I'll polish up the code and UI once that's stabilised.

The user can specify in their cosmos config their desired list of responsive devices under the key responsiveDevices. The default value for the moment is:

  responsiveDevices: [
    { label: 'iPhone 5', width: 320, height: 568 },
    { label: 'iPhone 6', width: 375, height: 667 },
    { label: 'iPhone 6 Plus', width: 414, height: 736 },
    { label: 'Medium', width: 1024, height: 768 },
    { label: 'Large', width: 1440, height: 900 },
    { label: '1080p', width: 1920, height: 1080 }
  ]

Any suggestions on a better list of defaults? Is there a standardised list of Android devices we can add? Bare in mind that the user can supply their own list, so our defaults don't have to be perfect.

We add a button to toggle the responsive loader:

screen shot 2017-12-17 at 6 37 56 pm

If you click on this, you will then see a new menu bar where the component normally shows up:

screen shot 2017-12-17 at 6 38 47 pm

Here we show all your specified devices. Clicking on these shows your component inside an appropriately sized iFrame. If the desired size does not fit on your screen, we scale the iFrame but keep the exact pixel dimensions. As part of "level 2", we will enable toggling scaled or not, but for now, it always makes the device size fit into your screen.

Any desired UX (not UI) changes to this new menu bar?

If you click on "Custom", you can then enter a custom dimension for your iFrame:
screen shot 2017-12-17 at 6 41 21 pm

One issue here is that we need to default Custom to some specific dimensions (showing 0 x 0 when you click on Custom isn't a nice UX). However, if the value that we default to actually exists in the responsiveDevices list, then clicking on Custom will break. Any ideas here? We could write a loop that starts with e.g. 400 x 400, checks if it exists already, adds a pixel, checks again, etc, but that doesn't seem particularly elegant. Alternatively I could add a forceCustom flag to the fixture.

We actually store the desired dimensions in the fixture, and update this when you click on a button in the responsive menu bar:

{
  "responsive": {
    "width": 400,
    "height": 400
  },
}

Any requests for a different key name than responsive?

This means you can also save your desired dimensions in your fixture source code. For example, you might have a component that shows two different things based on css media queries. This way you could save a "Component Desktop" and "Component Mobile" fixture that automatically set the desired window dimensions for you.

One outstanding issue is that the Responsive toggle button (the phone icon next to the full screen icon) stops working once your fixture has the responsive key in it, as we want to guarantee that the responsive mode is showing if you've got this specified in your fixture. Is this a big deal UX wise?

I think getting these basic issues cleaned up would be a great 'first release' of a responsive mode. @skidding would you agree, or would you want some other features?

In terms of future features:

  • Toggle whether or not to scale
  • Allow toggling device orientation (swap width and height values)
  • Allow showing multiple devices on screen at once
  • Anything else you'd like to see?

Thanks all.

@@ -40,7 +41,8 @@ type Props = {
component?: string,
fixture?: string,
editor?: boolean,
fullScreen?: boolean
fullScreen?: boolean,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skidding trailingComma: true in Prettier will prevent lines like this showing up in diffs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to make a PR ¯_(ツ)_/¯

@ovidiuch
Copy link
Member

From a personal perspective with regards to my available time to work on this, I'd like to publish small, iterative updates to this feature, rather than try to do everything possible in one massive PR.

Great, this is preferable. 👍

Any requests for a different key name than responsive?

How about viewport?

PS. It would be great if this property could also work in Jest/jsdom as well, somehow overriding window.width/height. I've done this once but I'm not sure how reliable it is, especially since the latest jsdom in Jest 22 seems breaking because it doesn't allow us to override stuff like window location anymore. Anyway, no need to worry about this, just something to consider. It's not uncommon to render different stuff based on viewport size so it would be useful to work in headless tests as well.

Any desired UX (not UI) changes to this new menu bar?

Maybe an apply button? It seems weird to change as I type. Eg. if the value is 0 and your selection is on the right you can't hit backspace to write a new number. Weird edge case but it's awkward.

However, if the value that we default to actually exists in the responsiveDevices list, then clicking on Custom will break

The apply button could fix this. A state flag that isn't part of the fixture could show/hide the custom options, but once you click apply the width/height gets applied to the fixture and the corresponding button will be highlighted, be it the custom option or a specific resolution.

One outstanding issue is that the Responsive toggle button (the phone icon next to the full screen icon) stops working once your fixture has the responsive key in it, as we want to guarantee that the responsive mode is showing if you've got this specified in your fixture. Is this a big deal UX wise?

I think this can be solved. The playground knows the serializable parts of the current fixture (and width/height are just numbers, so they will be received). The partial fixture body is received after fixture is selected and after every fixture update. Via fixtureLoad and fixtureUpdate events respectively. And the body of the current fixture is kept under ComponentPlayground.state.fixtureBody. So the new Responsive toggle can actually be driven by state.fixtureBody.viewport (or whatever we decide to call it). In this case, when you press on the Responsive button what should happen is the fixture is changed to contain a viewport field with an arbitrary resolution, and the user picks up from there.

I think getting these basic issues cleaned up would be a great 'first release' of a responsive mode. @skidding would you agree, or would you want some other features?

I agree. All other features are great but they can be added incrementally. Plus we'll gather more feedback once we start using it so we'll know what to focus on next.

@ovidiuch
Copy link
Member

Two details to consider:

  • The checkerboard is meant to reveal the parts the component that don't have a background (transparent). With this new additional frame I would argue that the checkerboard pattern should remain inside the viewport, and not around it. We can put a gray color or whatever looks good around it.
  • In the past I did my best to ensure that the Playground has the same element nesting even when fullscreen or with the fixture editor open, so that the Loader frame DOM element is preserved between transitions and the Loader content never flickers when opening panels or changing fixtures. I noticed that whenever you change a resolution the iframe gets destroyed and added to DOM again. If this is the case, is there a reason for this?

@maxsalven
Copy link
Collaborator Author

maxsalven commented Dec 24, 2017

The checkerboard is meant to reveal the parts the component that don't have a background (transparent). With this new additional frame I would argue that the checkerboard pattern should remain inside the viewport, and not around it. We can put a gray color or whatever looks good around it.

Done.

In the past I did my best to ensure that the Playground has the same element nesting even when fullscreen or with the fixture editor open, so that the Loader frame DOM element is preserved between transitions and the Loader content never flickers when opening panels or changing fixtures. I noticed that whenever you change a resolution the iframe gets destroyed and added to DOM again. If this is the case, is there a reason for this?

It's a bit messy as we have to nest 4 divs to get the layout correct for the responsive mode, but this is now fixed. I still see some occasional flicker with some of the more complex fixtures (but I get this in master too). If you look at my 'Demo' fixture, I see zero flicker.

I think this can be solved. The playground knows the serializable parts of the current fixture (and width/height are just numbers, so they will be received). The partial fixture body is received after fixture is selected and after every fixture update. Via fixtureLoad and fixtureUpdate events respectively. And the body of the current fixture is kept under ComponentPlayground.state.fixtureBody. So the new Responsive toggle can actually be driven by state.fixtureBody.viewport (or whatever we decide to call it). In this case, when you press on the Responsive button what should happen is the fixture is changed to contain a viewport field with an arbitrary resolution, and the user picks up from there.

Not sure I follow. At the moment, in order to decide whether to render the responsive tool, we check if the url has responsive=true or if there is a state.fixtureBody.viewport object. So if you define a fixture which has a viewport attribute, the responsive tool is always rendered even if you try to toggle it off via the button (which removes responsive=true from the URL). Is there an obvious solution to this?

If we purely have the 'show responsive layout' be a function of the state.fixtureBody.viewport, then when you switch between fixtures, the responsive layout might disappear. Whereas it should be sticky; if I'm in responsive view, switching between fixtures (including those without a specific viewport) should persist whatever responsive layout I've selected.

Maybe an apply button? It seems weird to change as I type. Eg. if the value is 0 and your selection is on the right you can't hit backspace to write a new number. Weird edge case but it's awkward.

I actually quite like the 'change as you type' style. Are you strongly against it? If not, then I can fix the 'can't delete 0' bug separately.

@ovidiuch
Copy link
Member

If we purely have the 'show responsive layout' be a function of the state.fixtureBody.viewport, then when you switch between fixtures, the responsive layout might disappear. Whereas it should be sticky; if I'm in responsive view, switching between fixtures (including those without a specific viewport) should persist whatever responsive layout I've selected.

This is a classic multiple source of truth problem. If we want state.fixtureBody.viewport to automatically trigger a responsive view (which I still think is a good idea), we should make it the single source of truth. And if we also want the responsive layout toggle to be sticky, maybe we could apply this logic:

After a fixture is selected (on fixtureLoad event), the Playground automatically adds the viewport field on the current fixture if the responsive URL param is true and if the fixture doesn't already have it (via fixtureEdit event). Fixture edits aren't persisted so no harm is done.

But this raises some questions:

  • Is the opposite flow problematic? User untoggles the responsive view and then clicks a fixture that has the viewport field, which re-enables the responsive view. Note that the "responsive" button in the Playground remains untoggled. Possible enhancements: Render a small responsive icon next to fixture name to make this more intuitive.
  • If the responsive mode is on (and sticky between fixtures), what viewport do we apply when we select a new fixture? The responsive=true URL param doesn't tell us the previous viewport width & height, which got lost when we discarded the prev fixture.

@ovidiuch
Copy link
Member

ovidiuch commented Feb 6, 2018

@maxsalven OK, I published this branch under the responsive npm tag to get the ball rolling. I skipped the git parts of the Lerna publish cmd (committing, tagging, and pushing git changes) to keep the branch cleaner. You can now run yarn add --dev react-cosmos@responsive in your project and work with this branch. Note that all other packages have also been published under this npm tag, but I don't think they are impacted in any way so you can still use their latest version.

@eltonio450
Copy link

Hello :)

Any update on this topic ? This looks super useful, but maintaining a fork of this branch is a bit overkill.

What is needed to merge with master ?

Thanks !

@ovidiuch
Copy link
Member

Hi @eltonio450,

I'm planning a bigger overhaul of the Cosmos UI which will surely include this feature, but I'm not sure if this branch will ever get merged or the code from it cherry picked instead. However, it's part of a bigger plan and the best estimation I can give is some time this summer.

@mpachin
Copy link
Contributor

mpachin commented May 25, 2018

@skidding, @maxsalven I think I can drop my five cents about this functionality.

Personally I can't get the idea of re-implementing something which already was solved by web devtools, and it was solved perfectly. Storybook for example offers lots of re-implemented functionality of chromes/firefox devtools and I don't get it, and I believe root of the problem is in its addons system.

Instead of thoughtful and careful steps towards DX (which is the main concern of discussed kind of tools), people start doing something in hope this 'something' will be used by others.

Please look at the main reason of creating Sizzy:

I was already using react-storybook to switch between all the variations of a component, but I still had to switch between 12 devices just to see the changes in all of them. And that’s how it all started.

How about making a tool where you can preview multiple screens at once as you work?

This is not devtools re-implementation, this is multiple-screens-problem elimination. This is the main reason this tool was created.

I strongly believe that if we should have such functionality in react-cosmos, then it needs to be multiple-screens-problem elimination. For specific resolutions development browsers devtools already matches ideally.

@ovidiuch
Copy link
Member

@mpachin You're right. Rendering multiple loaders with different resolutions at the same time is the end game, and I'm confident it will be part of Cosmos in the future.

In the meantime, however, we can't rely on the browser dev tools' responsive capabilities because they only apply to the top frame and even in full screen mode Cosmos loads the component inside an iframe.

We could fix this by tweaking the current full screen mode. Instead of rendering a chromeless playground with the loader iframe in full screen mode, we can instead render the loader iframe contents directly: http://host:port/_loader.html?component=x&fixture=y. This is going to make the transition between regular and full screen modes less subtle, because the playground will reload completely when we go back, but it will allows us to use the entire set of responsive capabilities of any browser's dev tools.

Wdyt?

@maxsalven
Copy link
Collaborator Author

Hi all, great to have some discussion on this PR!

A few thoughts:

My personal workflow isn't one where I care about multiple screens at once. I'm typically in a 'mobile' context or a 'desktop' context whilst developing, and prefer to be focused purely on one version at a time. Having the two dimensions on screen at the same time, on my laptop along with the Cosmos file directory on the left (I'm very frequently switching between fixtures) would render the desktop view so small as to not be usable (or if it wasn't scaled to fit, I'd have to scroll all the time). Instead, I personally want to see just one view at a time. Just my personal use case, of course others may flow differently!

We could fix this by tweaking the current full screen mode.

I think switching between fixtures might be a bit clunky in this situation? Exit full screen, exit Chrome responsive mode, pick new fixture, enter full screen, enter Chrome responsive mode.

I've actually been using this 'responsive cosmos' fork exclusively for the past few months, and it's worked really well for me. I have full access to the file list, and I can toggle into and out of mobile mode with a single click, and very easily switch between device sizes too.

@skidding if I updated this PR to match the latest master branch, would it still be considered for inclusion? I've got one remaining TODO which is to make a toggle for the 'scaled' mode setting. Full disclosure, there is also one bug, which I will try to fix: Occasionally, upon restarting Cosmos and only if you were in the responsive mode already, then it starts up showing your component in a tiny viewport and upside-down. Reloading the window will immediately fix this. It's a bit of a niche scenario, as I don't think the latest cosmos ever needs to be restarted anymore (as opposed to the older release, which needed a manual restart when adding a brand new component fixture).

@ovidiuch
Copy link
Member

Instead, I personally want to see just one view at a time. Just my personal use case, of course others may flow differently!

Good point. I guess Sizzy covers another use case, which is a birds eye view post CSS changes, especially useful for small sized presentational components. But I agree this would be a secondary use case and the primary one is likely focused around a single resolution at a time.

I think switching between fixtures might be a bit clunky in this situation? Exit full screen, exit Chrome responsive mode, pick new fixture, enter full screen, enter Chrome responsive mode

Correct. This would be an escape hatch.

@skidding if I updated this PR to match the latest master branch, would it still be considered for inclusion?

Definitely.

Occasionally, upon restarting Cosmos and only if you were in the responsive mode already, then it starts up showing your component in a tiny viewport and upside-down.

I remember experiencing a bug where the viewport would become 0x0 until hard refresh. There might be more than one bug to fix, but I'm happy to help debug and maybe more people want to help beta test once the branch is brought up to date.

@maxsalven
Copy link
Collaborator Author

There might be more than one bug to fix

Indeed, sorry I should have been clearer, 'one bug that I've encountered' :)

Definitely.

Great, I'll find some time next week to freshen things up, and take a look at the testing side, which hasn't happened yet.

@maxsalven
Copy link
Collaborator Author

I've managed to get this up and running on the latest master. Unfortunately the 'dimensions are the wrong size on very first load' issue has been a struggle, will try again soon.

@maxsalven
Copy link
Collaborator Author

@skidding, as discussed on Slack, if you could take a look at the scaling bug that would be great!

To reproduce, simply open a new Chrome window, go to http://localhost:8090 and then select ResponsiveLoader > Demo > Demo - iPhone 6 Viewport. You should see a really tiny component that's upside down (because it has incorrectly calculated the scaling required as * -0.09). If you then hit refresh, everything will be fine for the remainder of your Cosmos session. Thanks.

@ovidiuch
Copy link
Member

I think I found the issue with the tiny viewport. Here's a few logical steps that lead me to my conclusion:

  • The loader's width/height are calculated on mount (inside updateContainerDimensions)
  • The loader is always mounted, even on the home page (that's how we communicate with the user bundle)
  • On the home page the loader is hidden (thus 0x0 size)
  • When we open a fixture, we need to re-trigger the viewport calculations because that's when the loader gets visible and gains width/height
  • This is why the responsive viewport works OK when we open a fixture directly but not through home page

Haven't explored in depth, but here's a possible solution: Only send the fixture prop to ResponsiveLoader when a fixture is selected in ComponentPlayground. In ResponsiveLoader, you could check if !prevProps.fixture and props.fixture and call updateContainerDimensions. Note that I'm not familiar with all code from this branch so you might think of a more elegant solution.

@maxsalven
Copy link
Collaborator Author

Thanks @skidding, that was indeed the issue! Fixed in 38e8fa7

@ovidiuch
Copy link
Member

Branch is looking good! Here are my thoughts.

Responsive modes can cause overflow to the entire Cosmos UI:

responsive-overflow

Solution ideas:

  • More favorable: Take both width and height into account when scaling
  • Less favorable: Overflow the ResponsiveLoader area only

Here's a weird glitch:

responsive-glitch

After responsive=forceHide is set on a fixture with viewport, moving to another fixture without a viewport shows the responsive chrome for a second and then closes it.

UI territory:

  • In the future we'll have a scalable menu approach, but for now I say we increase the left nav's min width to 264px to accommodate the new button
  • Maybe put the responsive button in the middle, or first?

Other notes:

  • The ComponentPlayground fixtures you added need to be updated with props.options.platform: 'web'.
  • No need to worry about tests (unless you feel like writing some), as long as the existing ones pass. As we've talked I want to make the UI pluggable so I'll likely dissect existing pieces to make them core plugins in the process.

The last thought I have is related to the design of the responsive URL param and the affiliated state. I respect that this has a lot of implication and the cost of changing the implementation might be higher than the benefit. But if you have a few minutes I would like to explore an alternative and at least come up with clear comparison between the approaches.

Here's a sample of code that makes me uncomfortable:

// We don't persist the `forceHide` value when changing fixturess
responsive: responsive === 'forceHide' ? false : responsive
});
const showResponsiveControls =
responsive === 'forceHide' ? false : fixtureBody.viewport || responsive;
const nextResponsive =
responsive === 'forceHide'
? true
: fixtureBody.viewport
? 'forceHide'
: !responsive;

I get it after reading it a few times, but it's not quick to understand and the mixed boolean/string value definitely adds to the confusion.

Again, this might just make the current solution more clear in my head, but I'd like to explore an alternative solution and see if it at least matches the current functionality. Here's the gist:

  • Responsive state is kept in/derived from URL. It can be viewport=400,300 or anything else that makes more sense.
  • Selecting a fixture with viewport defined appends the viewport URL param
  • Clicking the responsive button either sets this URL param (gets values from current fixture or local storage), or clears it
  • The ResponsiveLoader/Header inputs are uncontrolled, and update the URL on change when the values are valid (eg. not an empty string), as well as updating the fixture and local storage, which it already does
  • Until we have a toggle for scaling, I believe scaling is always-on and can be derived without additional state

How do you think the current approach compares to the existing one?

Thanks!

Conflicts:
	packages/react-cosmos-playground/src/components/ComponentPlayground/index.js
@maxsalven
Copy link
Collaborator Author

Responsive modes can cause overflow to the entire Cosmos UI:
Solution ideas:
More favorable: Take both width and height into account when scaling

The UI should default to scaled, which should never overflow the UI. Only if you explicitly untoggle scaling will we should a component bigger than the available screen space. Is this acceptable?

screen shot 2018-07-06 at 3 55 59 pm

In the future we'll have a scalable menu approach, but for now I say we increase the left nav's min width to 264px to accommodate the new button

I have now changed

@left-nav-min-width: @header-button-size * 3 + @default-spacing * 4;

to

@left-nav-min-width: @header-button-size * 4 + @default-spacing * 5;

Which equated to 254. LMK if you prefer different multipliers.

Maybe put the responsive button in the middle, or first?

Have moved it to the middle.

The ComponentPlayground fixtures you added need to be updated with props.options.platform: 'web'.

Done.

How do you think the current approach compares to the existing one?

I'd like to explore this, but the first hurdle is that ComponentPlayground seems to only receive a string based map of ComponentName: Array<fixtureName>; in order to have the URL include viewport params, we'll need to have access to the actual fixture objects rather than just the string names I imagine? Is this going to be difficult?

@ovidiuch ovidiuch merged commit bae8f73 into master Jul 7, 2018
@ovidiuch ovidiuch deleted the feature-responsive-layout branch July 7, 2018 05:27
@ovidiuch
Copy link
Member

ovidiuch commented Jul 7, 2018

@maxsalven Thanks for all the effort! Merged six months later 🎉

I'll make some tweaks starting from latest master and then publish a new pre release.

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

Successfully merging this pull request may close these issues.

None yet

5 participants