Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

feat: add tab switch to hide pages and sidebar #560

Merged
merged 6 commits into from
Jun 14, 2018
Merged

Conversation

tilmx
Copy link
Member

@tilmx tilmx commented Jun 13, 2018

Add a tab switch to hide both the pages or the elements & library:

bildschirmfoto 2018-06-13 um 21 39 19

@markusoelhafen
Copy link
Member

I don't see the necessity for the underline to highlight the active icon in addition to the color

@tilmx
Copy link
Member Author

tilmx commented Jun 13, 2018

It makes more sense when this pattern is used on the right side as a sidebar switch to switch between Properties and Project Settings, doesn’t it?

bildschirmfoto 2018-06-13 um 21 45 00

@markusoelhafen
Copy link
Member

Also, if there are options to toggle pages and the elementpane, there should also be an option to hide the property pane

@markusoelhafen
Copy link
Member

@tilmx if it's used as an actual tab I agree, but if it's used as an checkbox I'd disagree.
In tabs, the line might helps to see the switch between two views in the same pane.

@tilmx
Copy link
Member Author

tilmx commented Jun 14, 2018

I removed it.

Copy link
Contributor

@marionebl marionebl left a comment

Choose a reason for hiding this comment

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

Some minor comments

import styled from 'styled-components';

export interface TabSwitchProps {
active?: boolean;
Copy link
Contributor

@marionebl marionebl Jun 14, 2018

Choose a reason for hiding this comment

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

Let's use an required enum instead:

enum TabSwitchState {
  Active,
  Default
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Did that!

@@ -508,7 +507,7 @@ async function createWindow(): Promise<void> {
minWidth: 780,
minHeight: 380,
titleBarStyle: 'hiddenInset',
backgroundColor: Color.Grey97,
backgroundColor: '#f7f7f7',
Copy link
Contributor

Choose a reason for hiding this comment

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

Some special reason for this one?

Copy link
Member Author

Choose a reason for hiding this comment

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

Color.Grey97 didn’t work (I guess because electron only allows hex values)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so lets use the color module here to convert from rgb to hex then.

marionebl
marionebl previously approved these changes Jun 14, 2018
marionebl
marionebl previously approved these changes Jun 14, 2018
@marionebl marionebl merged commit 2aba3b8 into master Jun 14, 2018
@marionebl marionebl deleted the feat/add-tab-switch branch June 14, 2018 20:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants