Add "onion with master"

[TejasQ]

Similar to how we have diff with master on pull request previews, let’s try and have an “onion mode” where:

• if someone is looking at a PR preview or local host
  • show “onion” button

On click of the onion button, we open up an iFrame with master and overlay it on top of the current page with 0.5 opacity.

The scroll bars of the iFrame and the actual page content will need to somehow be linked. This will likely be the hardest part.

[b14s]

Spent some time orienting myself with the source code. I just traced out the StyleguideContext -> Header dependency and got my head around the state changes with dispatch.

I assume PR preview and 'deploy-preview' are interchangeable, but are there any specific components relevant to it? Or is it dependent on 'isDiffWithMaster' like localhost and there are no UI/code differences?

[TejasQ]

I assume PR preview and 'deploy-preview' are interchangeable

correct.

but are there any specific components relevant to it?

The IFrame for example is relevant to it. I'm not sure I fully understand the question.

[b14s]

I did overlook IFrame in my comment, but I had taken a look at, I guess I meant are there any other components relevant to PR previews that I had overlooked tucked away somewhere, but I take it not?

If that's the case I have a very clear concept of my plan of attack, so thank you for your help clarifying!

[TejasQ]

Awesome! @Corvinae, what is your plan of attack?

[b14s]

@TejasQ, well the original plan was a little opinionated so I figured I'd ask more questions. 😄

Is "onion mode" intended to be completely seperate from "diff from master", ie seperate buttons/iframes, or is the onion button intended to be a mode of the diff view, ie seperate buttons, same iframe, but with style changes/transformation to accomplish overlay?

My original plan was more in line with the latter, but rereading the original issue made me reconsider. I'm not quite sure about the scrolling issue, whether it can be fixed with overflow properties or requires some runtime sync? It's a rough plan of attack. I'm excited to try it out.

[b14s]

I feel like "onion mode" as a suboption of the diff view is more intuitive, and restricts the user from bringing the two views into conflict without requiring extensive conditional rendering.

[TejasQ]

Is "onion mode" intended to be completely seperate from "diff from master", ie seperate buttons/iframes, or is the onion button intended to be a mode of the diff view, ie seperate buttons, same iframe, but with style changes/transformation to accomplish overlay?

Good question. I think it would make more sense for encapsulation (of UX) if the onion button was a mode of the diff view, which means we either:

• diff by split screen, or
• diff by onion overlay

That would be the cleanest imho. What do you think? I think it would be really interesting to figure out scroll and event propagation across the onion layer and the main layer. 😍

[b14s]

@TejasQ, I agree, exclusive views is what I had in mind. That way there's one button to open, which gives way to two buttons: close and switch mode. I have another question if you don't mind. 😆

I have a second button rendering in Header's end prop when "diff mode" is on, but the lack of horizontal space in HeaderBar::End leads to the "close diff" text wrapping vertically. This requires either different placement of the onion button, or some style changes to the parent/parents' siblings (HeaderBar's End/Start/Center).

If a modification to a src/ component is required, is it best to extend that component with a new declaration where it's needed in styleguide/, ie extending 'styled(HeaderBar)' in Header.tsx? My idea is to change HeaderBar from grid to flex, set flex-grow for the Center and inline display/width 100% on the End, or something else that will dynamically size the Center of HeaderBar to allow arbitrary amounts of children in End.

I'm hesitant to propagate changes in src/ where the functionality is relevant only to the styleguide, but I think there might be a broad usecase for adding dynamic styling to HeaderBar if it is intended to accept arbitrary encapsulation by end-users.

[TejasQ]

If a modification to a src/ component is required, is it best to extend that component with a new declaration where it's needed in styleguide/, ie extending 'styled(HeaderBar)' in Header.tsx?

It depends. Normally, I'd say yes.

I'm hesitant to propagate changes in src/ where the functionality is relevant only to the styleguide

This tells me you're a great engineer. 😍

I suggest actually making the change to src because we're a little bit too strict about HeaderBar's end. I think switching from grid to flex isn't necessary, but simply replacing the size of the end column, (250px) with max-content should give us the flexible behavior we're looking for.

Perhaps try that out and see if it fits your requirements?

[TejasQ]

What's up, @Corvinae? How's it looking so far?