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

fix(UI Pattern): design review fixes #731

Merged
merged 18 commits into from
Feb 1, 2024
Merged

Conversation

SamCreasey
Copy link
Contributor

@SamCreasey SamCreasey commented Jan 30, 2024

Task: CU-86933q1m6

Requires guideline-blocks-settings bump

Fixes:

  • Border radius has been removed from child components and is now controlled by the wrapper with overflow: hidden.
  • Accessibility styles have more consistent visibility with inset styles, so they are always visible in view and edit mode when required
  • Dependency settings have been moved to "Basics" and default values set to false
  • Theme settings have been moved up in the style settings, and label for background has been updated
  • Min-height has been added to Code Editor to reduce the visibility of flickering from content unmounting and re-mounting (Issue reported)
  • Border-bottom removed from Code Editor when there are no dependencies visible in view mode to prevent a double-border from appearing
  • If there is no user-applied border then border-bottom remains visible for Code Editor and dependencies

@SamCreasey SamCreasey changed the title fix: codesandbox fixes fix(UI Pattern): design review fixes Jan 30, 2024
@SamCreasey SamCreasey marked this pull request as ready for review January 30, 2024 14:14
@SamCreasey SamCreasey requested a review from a team as a code owner January 30, 2024 14:14
@SamCreasey SamCreasey requested review from silviojaeger and ragi96 and removed request for silviojaeger January 30, 2024 14:14
Copy link
Collaborator

@ragi96 ragi96 left a comment

Choose a reason for hiding this comment

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

Just an issue that i found: if you change the theme it just takes effect after a reload and not directly

@SamCreasey
Copy link
Contributor Author

Just an issue that i found: if you change the theme it just takes effect after a reload and not directly

Unfortunately the only way to fix this is to rerender the whole SandpackProvider, as the Theme Provider is nested inside and it sets an internal state from the theme prop.
https://github.com/codesandbox/sandpack/blob/main/sandpack-react/src/styles/themeContext.tsx

I have fixed the code accordingly

Copy link
Collaborator

@ragi96 ragi96 left a comment

Choose a reason for hiding this comment

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

Works now as expected

@SamCreasey SamCreasey merged commit c82e55c into main Feb 1, 2024
10 checks passed
@SamCreasey SamCreasey deleted the fix/codesandbox-fixes branch February 1, 2024 09:03
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

2 participants