-
Notifications
You must be signed in to change notification settings - Fork 351
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
Frame header #1159
Frame header #1159
Conversation
87e1e2c
to
fcf4f90
Compare
@@ -47,7 +47,9 @@ | |||
"prettier/@typescript-eslint" | |||
], | |||
"plugins": ["@typescript-eslint/eslint-plugin"], | |||
"rules": {} | |||
"rules": { | |||
"@typescript-eslint/explicit-function-return-type": "off" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Up for discussion if we want to keep this, I liked not needing to write return type on components mostly.
@@ -33,11 +33,16 @@ export const CloseButton = props => { | |||
} | |||
|
|||
export const EditorButton = props => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the editorbuttons flexible in size
export { withBus, BusProvider, BusProps } | ||
} | ||
|
||
declare module 'suber' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a start for typing up suber and react-suber. Some of the any
s are unavoidable sadly. Perhaps we could do something with generics but not sure if it's worth it.
@@ -33,8 +34,8 @@ interface ActionButton { | |||
iconColor?: string | |||
} | |||
|
|||
const ActionButtons: React.FC<ActionButtonProps> = ({ buttons }) => ( | |||
<ActionButtonSection containerWidth={buttons.length * 33}> | |||
const ActionButtons = ({ buttons, width = 24 }: ActionButtonProps) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved away from React.FC after reading this: facebook/create-react-app#8177
@@ -80,7 +80,6 @@ export class Editor extends Component { | |||
buffer: '', | |||
mode: 'cypher', | |||
notifications: [], | |||
expanded: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
full screen state lifted to new component
message: message | ||
.split('\n') | ||
.filter((nonEmpty: string) => nonEmpty) | ||
.join(' '), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New behaviour! Like we talked about it makes no sense to show a collapse-button if you can't use it. To me it felt natural to join the lines together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check with ABK if this is OK
} | ||
return "'editor header'" | ||
}}; | ||
grid-template-areas: 'editor header'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could perhaps be simpler with flexbox. I think it's good enough
// 230 is 10 lines + 2*12px padding | ||
return '254px' | ||
} | ||
return '0' | ||
}}; | ||
|
||
${(props): string => (props.expanded ? '' : 'transition-duration: 0.1s;')} | ||
transition-duration: 0.1s; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked buggy before when changing color on card view, the check is not needed anymore
background-color: ${props => props.theme.secondaryBackground}; | ||
margin: ${editorPadding}px 0px ${editorPadding}px 0; | ||
border-radius: 2px; | ||
box-shadow: 0px 0px 2px rgba(52, 58, 67, 0.1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it looked too flat, got this box-shadow from liza. Thoughts?
@@ -50,6 +49,9 @@ export const StyledFrame = styled.article` | |||
&:hover .carousel-intro-animation { | |||
opacity: 0; | |||
} | |||
box-shadow: 0px 0px 2px rgba(52, 58, 67, 0.1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use "standardized" box shadow from designsystem
c91da01
to
ae4ecfe
Compare
message: message | ||
.split('\n') | ||
.filter((nonEmpty: string) => nonEmpty) | ||
.join(' '), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check with ABK if this is OK
Adds UI controls to the editor. Details in the comments
Gif demo of new looks
changelog: Make editor a frame that has control buttons