Skip to content

Conversation

@siddharthkp
Copy link
Contributor

@siddharthkp siddharthkp commented Oct 11, 2019

The goal of this PR is to move closer to the final design without making the experience worse. (n + 1 > n)

Built on top of #2678

Part 1

Changes the color of the shell without changing the components. The goal of this PR is to move closer to the final design without making the experience worse. (n + 1 > n)

Implementation detail: Created a new VSCode theme that matches our design

Using it in embed. Had to modify a few components (2) to use the right keys

Part 2

Changes the structure of the sidebar by adding foldable sections + redesigned the sandbox info tab

Added stats to the sandbox info section

Demo

https://pr2731.build.csb.dev/embed/5pj49

What steps did you take to test this?

  • Testing embed applies new styles
  • Testing sandbox remains the same

Checklist

  • [NA] Documentation
  • Testing
  • Ready to be merged
  • [NA] Added myself to contributors table

@vercel
Copy link

vercel bot commented Oct 11, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/codesandbox/codesandbox-client/p96it9lj3
🌍 Preview: https://codesandbox-client-git-embed-sidebar.codesandbox1.now.sh

@siddharthkp siddharthkp changed the base branch from master to embed-black October 11, 2019 13:28
@vercel vercel bot temporarily deployed to staging October 11, 2019 15:07 Inactive
@vercel vercel bot temporarily deployed to staging October 14, 2019 09:38 Inactive
@siddharthkp siddharthkp changed the title Embed redesign 2/n: Sidebar Embed redesign 2/n: Sidebar sections + Sandbox info Oct 14, 2019
@siddharthkp
Copy link
Contributor Author

@DannyRuchtie Quick question:

The redesign also introduces stats on an embedded sandbox. Should clicking on the like/fork icon trigger an action?

(for comparison, the heart on app triggers a like, the fork doesn't do anything)

Screenshot 2019-10-14 at 11 44 25 AM

@DannyRuchtie
Copy link
Contributor

It would be nice!

@siddharthkp
Copy link
Contributor Author

siddharthkp commented Oct 14, 2019

It would be nice!

  • Good to have?

or

  • It's silly to show the icons without an action because it's breaks expectation

@vercel vercel bot temporarily deployed to staging October 14, 2019 09:50 Inactive
@DannyRuchtie
Copy link
Contributor

Let's say it's nice to have.
Not prio #1 but a really nice to have!
currently, we don't have the actions (explore, and viewing someone's sandbox)

Schermafbeelding 2019-10-14 om 11 51 58

Schermafbeelding 2019-10-14 om 11 51 47

@siddharthkp siddharthkp changed the base branch from embed-black to master October 14, 2019 09:56
@vercel vercel bot temporarily deployed to staging October 14, 2019 10:15 Inactive
@vercel vercel bot temporarily deployed to staging October 14, 2019 11:26 Inactive
@vercel vercel bot temporarily deployed to staging October 14, 2019 11:58 Inactive
@vercel vercel bot temporarily deployed to staging October 14, 2019 12:04 Inactive
@siddharthkp siddharthkp mentioned this pull request Oct 15, 2019
4 tasks
@siddharthkp siddharthkp marked this pull request as ready for review October 15, 2019 08:23
@siddharthkp
Copy link
Contributor Author

@DannyRuchtie Does this look like a good n+1 step to ship?

@CompuIves
Copy link
Member

CompuIves commented Oct 15, 2019

This looks awesome!! A couple things I'd like some discussion on:

  1. Do we want to open the files item in the sidebar by default?
  2. Does it make sense to have something on hover for the file tabs (in the editor)?

cc @DannyRuchtie

height: 35px;
min-height: 35px;
background-color: rgba(0, 0, 0, 0.3);
background-color: ${props => props.theme['tab.inactiveBackground']};
Copy link
Member

Choose a reason for hiding this comment

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

Nice that we're using themes here already!

@DannyRuchtie
Copy link
Contributor

DannyRuchtie commented Oct 15, 2019

1) Do we want to open the files item in the sidebar by default?

Makes sense. But because of the big open in Codesandbox button and logo. It currently looks a bit cluttered.

_

  1. Does it make sense to have something on hover for the file tabs (in the editor)?

_
I agree animate font color to white would be nice

Two things:

  1. The tabbar height seems to be 35px (should be aligned with the sidebar headers)

Schermafbeelding 2019-10-15 om 16 39 09

  1. Font size of the name is 12px (was 11px to small?)

Schermafbeelding 2019-10-15 om 16 41 18

@siddharthkp
Copy link
Contributor Author

siddharthkp commented Oct 15, 2019

I agree animate font color to white would be nice

sounds good 👍

The tabbar height seems to be 35px (should be aligned with the sidebar headers)

Yep, haven't made my way to the tab bars yet. Does it look worse than what we have on production? Might make sense to wait for consistent heights before merging?

Font size of the name is 12px (was 11px to small?)

Of the sandbox name? I copied from the IDE file where it's 12. Just checked the Embed file to cross check and it's 11px there 😅. Can reduce that to 11

@siddharthkp siddharthkp mentioned this pull request Oct 18, 2019
4 tasks
@siddharthkp
Copy link
Contributor Author

Superseded by #2791

@MichaelDeBoey MichaelDeBoey deleted the embed-sidebar branch December 16, 2019 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants