feat: improve user experience #607
Conversation
tilmx
changed the title
| @@ -403,6 +406,15 @@ export class ElementList extends React.Component { | |||
| > | |||
| <Components.Element.ElementChildren> | |||
| {childContent ? <ElementContentContainer content={childContent} /> : null} | |||
| {hasChildren ? ( | |||
There was a problem hiding this comment.
Should be simplified to
!hasChildren && (<Components.EmptyState ... />
src/components/add-button/index.tsx
Outdated
| border-radius: 6px; | ||
| background-color: transparent; | ||
| margin: ${getSpace(SpaceSize.XS)}px; | ||
| margin: ${(props: AddButtonProps) => (props.margin ? `${getSpace(SpaceSize.XS)}px` : '0')}; |
There was a problem hiding this comment.
Could the decision wether margin should be used placed outside the component via <Space/>?
src/components/add-button/index.tsx
Outdated
| export const AddPageButton: React.SFC<AddPageButtonProps> = props => ( | ||
| <StyledAddPageButton onClick={props.onClick}> | ||
| export const AddButton: React.SFC<AddButtonProps> = props => ( | ||
| <StyledAddButton margin={props.margin} text={props.text} onClick={props.onClick}> |
There was a problem hiding this comment.
Remove the text prop here and use a distinct StyledAddButtonInterface instead
There was a problem hiding this comment.
Just out of curiousity: Why is it better here to split the prop interfaces?
There was a problem hiding this comment.
I removed the text property and instead set the text as a children now.
|
|
||
| export const ChromeButton: React.StatelessComponent<ChromeButtonProps> = props => ( | ||
| <StyledChromeButton {...props}> | ||
| {props.icon ? <StyledIcon>{props.icon}</StyledIcon> : ''} |
There was a problem hiding this comment.
Simplify to props.icon && <StyledIcon .../>
src/components/copy/index.tsx
Outdated
| @@ -21,6 +21,8 @@ export enum CopySize { | |||
| const StyledCopy = styled.p` | |||
| margin: 0; | |||
| line-height: 1.5; | |||
| user-select: none; | |||
There was a problem hiding this comment.
I tried to imitate the native behaviour of an native App with that... Do you think it’s not a good idea?
There was a problem hiding this comment.
Its the discussion from #519 again, si?
Personally I am annoyed when apps prevent me from copying things. E.g. I might want to explain what part of the UI I am writing about by citing the text I found there. Being able to copy such text makes my life as a user considerably easier then.
There was a problem hiding this comment.
That being said there are legit use cases for user-select: none, e.g. draggable ui elements
src/components/headline/index.tsx
Outdated
| @@ -21,6 +21,8 @@ const StyledHeadline = styled.div` | |||
| margin-top: 0; | |||
| font-family: ${fonts().NORMAL_FONT}; | |||
| font-weight: 500; | |||
| user-select: none; | |||
|
@marionebl Thanks! I addressed all the issues. |
Contains:
General
Show short element drop animation on hoverProject Settings