Add Edit in Playground #72
Conversation
|
@mirisuzanne or @stacyk This is ready for design |
✅ Deploy Preview for sass-site-oddbird ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@jgerigmeyer I think this is ready for a code review |
There was a problem hiding this comment.
Looks good! Three questions:
- Visually, the "Edit on Playground" text looks bigger than what I see in the mockup. Should it be the same size as the Sass/SCSS/CSS tabs?
- The rectangle background on hover over "Edit on Playground" looks a bit strange to me, but I guess that's based on the existing link hover pattern. Maybe it will look better if the font size is a bit smaller.
- On narrower screens, do we want to hide the "Edit on Playground" link when the CSS tab is selected? Right now it opens the playground with the code in whichever style (Sass or SCSS) was most recently selected -- which is probably okay?
source/assets/js/playground/utils.ts
Outdated
| state: Partial<PlaygroundState> & | ||
| Pick<PlaygroundState, 'inputFormat' | 'outputFormat' | 'inputValue'> |
There was a problem hiding this comment.
I think this could be simplified?
| state: Partial<PlaygroundState> & | |
| Pick<PlaygroundState, 'inputFormat' | 'outputFormat' | 'inputValue'> | |
| state: Pick<PlaygroundState, 'inputFormat' | 'outputFormat' | 'inputValue'> |
There was a problem hiding this comment.
Hmm, that was so the function could be called with the whole state or a subset, but I can't track down where the whole state was being passed, so I took your suggestion.
There was a problem hiding this comment.
Oh, makes sense. I think the entire state is passed in earlier in this same file, but I also think TS allows extra properties on objects as long as the specified ones match.
This makes sense to me, but we should probably draw Natalie's attention to this and see if she has another proposal. |
@jgerigmeyer Good call, I updated it to match the tab size better.
You are right, these are the link hover styles minus the bottom border. I think it looks better smaller, and I thought it probably did need to look a bit different from the tab styles since it does a different thing than the tabs.
I think I still like the edit link shown even if on a css panel. There's still sass there, even if not shown at the moment. |
|
The hover stat isn't working for me anymore in Chrome... hold off on merging for a bit |
Nevermind, was an issue with my browser. Proceed if/when ready |
|
Moved to sass#1167 |
Original Design:
https://xd.adobe.com/view/84c872fe-0e51-46e4-a59c-f72aab2b8c6a-d19b/
I decided to not show a link if there are multiple sections of code, for instance on the @use page. Those examples require a importing and a file system, which isn't supported.
Todo: