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.

Sign up for GitHub

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

Jump to bottom

[Tabs] M3-4119: Refactor Tabs - Lish #6467

Merged
merged 9 commits into from Jun 22, 2020
Merged

[Tabs] M3-4119: Refactor Tabs - Lish #6467

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
38e46d9
Refactor to Reach UI
tiffwong May 22, 2020
f01e664
Add keyboardActivation prop
tiffwong May 26, 2020
9f28f36
Working on focusing on tabs
tiffwong May 27, 2020
c92009b
Add ref to tabs
tiffwong May 29, 2020
51df808
removing redundant a11y attributes
Jun 3, 2020
c77d2dc
Merge branch 'feature/tab-refactor' of github.com:linode/manager into…
tiffwong Jun 11, 2020
f1a99fc
Switch to SafeTabPanel
tiffwong Jun 11, 2020
89248f3
Move styles to Lish and get rid of ref
tiffwong Jun 12, 2020
c8020ce
Remove TabPanel import
tiffwong Jun 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 49 additions & 14 deletions packages/manager/src/components/TabLinkList/TabLinkList.tsx
View file
Open in desktop
Expand Up @@ -8,24 +8,24 @@ const useStyles = makeStyles((theme: Theme) => ({
tab: {
'&[data-reach-tab]': {
// This was copied over from our MuiTab styling in themeFactory. Some of this could probably be cleaned up.
color: theme.color.tableHeaderText,
minWidth: 50,
textTransform: 'inherit',
fontSize: '0.93rem',
padding: '6px 16px',
position: 'relative',
overflow: 'hidden',
maxWidth: 264,
boxSizing: 'border-box',
borderBottom: '2px solid transparent',
minHeight: theme.spacing(1) * 6,
flexShrink: 0,
display: 'inline-flex',
alignItems: 'center',
flexShrink: 0,
verticalAlign: 'middle',
justifyContent: 'center',
appearance: 'none',
borderBottom: '2px solid transparent',
boxSizing: 'border-box',
color: theme.color.tableHeaderText,
fontSize: '0.93rem',
lineHeight: 1.3,
maxWidth: 264,
minHeight: theme.spacing(1) * 6,
minWidth: 50,
overflow: 'hidden',
padding: '6px 16px',
position: 'relative',
textTransform: 'inherit',
[theme.breakpoints.up('md')]: {
minWidth: 75
},
Expand All @@ -40,6 +40,7 @@ const useStyles = makeStyles((theme: Theme) => ({
}
},
tabList: {
color: theme.color.tableHeaderText,
'&[data-reach-tab-list]': {
background: 'none !important',
boxShadow: `inset 0 -1px 0 ${theme.color.border2}`,
Expand All @@ -49,6 +50,30 @@ const useStyles = makeStyles((theme: Theme) => ({
padding: 1
}
}
},
lishTab: {
backgroundColor: theme.bg.offWhite,
color: theme.color.tableHeaderText,
'&[aria-selected="true"]': {
backgroundColor: theme.palette.primary.main,
borderBottom: 'none !important',
color: 'white !important',
'&:hover': {
backgroundColor: theme.palette.primary.light,
color: 'white'
}
}
},
lishTabList: {
display: 'flex',
backgroundColor: theme.bg.offWhite,
margin: 0,
overflow: 'hidden',
'& [role="tab"]': {
flexBasis: '50%',
margin: 0,
maxWidth: 'none !important'
}
}
}));

Expand All @@ -60,6 +85,7 @@ export interface Tab {
interface Props {
tabs: Tab[];
[index: string]: any;
lish?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of having TabLinkList handle the CSS for Lish, it'd be more scalable if it could accept a CSS classname so it's controllable by the parent. An example can be found here: https://github.com/linode/manager/blob/develop/packages/manager/src/components/PromotionalOfferCard/PromotionalOfferCard.tsx#L91

}

type CombinedProps = Props;
Expand All @@ -68,15 +94,24 @@ export const TabLinkList: React.FC<CombinedProps> = props => {
const { tabs } = props;

const classes = useStyles();
// TODO: Change type 'any' to a more appropriate type
const ref = React.useRef<any>(null);

React.useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, if this Ref is only here for Lish (for now), it should be controlled by the parent and passed down as a prop.

if (ref.current !== null) {
ref.current.focus();
}
}, []);

return (
<TabList className={classes.tabList}>
<TabList className={props.lish ? classes.lishTabList : classes.tabList}>
{tabs.map((tab, _index) => (
<Tab
className={classes.tab}
className={`${classes.tab} ${props.lish ? classes.lishTab : ''}`}
key={`tab-${_index}`}
as={Link}
to={tab.routeName}
ref={props.lish && _index == 0 ? ref : null}
>
{tab.title}
</Tab>
Expand Down
71 changes: 34 additions & 37 deletions packages/manager/src/features/Lish/Glish.tsx
View file
Open in desktop
Expand Up @@ -188,7 +188,6 @@ class Glish extends React.Component<CombinedProps, State> {
[classes.link]: true
})}
onClick={this.linodeOnClick(linodeID)}
role="button"
title={linodeLabel}
>
{linodeLabel}
Expand Down Expand Up @@ -314,43 +313,41 @@ class Glish extends React.Component<CombinedProps, State> {
}

return (
<div id="tabpanel-glish" role="tabpanel" aria-labelledby="tab-glish">
<div id="Glish">
{!powered && (
<div className={classes.errorState}>
<ErrorState errorText="Please power on your Linode to use Glish" />
</div>
)}

{/*
* The loading states have to render with the VncDisplay component
* because the messages from the websocket connection have to be send
* if you're rendering a loading state, then get a message from websockets,
* then render the VncDisplay, you end up with a blank black screen
*/}
{powered && !initialConnect ? (
isRetryingConnection ? (
this.renderRetryState()
) : (
<CircleProgress noInner />
)
<div id="Glish">
{!powered && (
<div className={classes.errorState}>
<ErrorState errorText="Please power on your Linode to use Glish" />
</div>
)}

{/*
* The loading states have to render with the VncDisplay component
* because the messages from the websocket connection have to be send
* if you're rendering a loading state, then get a message from websockets,
* then render the VncDisplay, you end up with a blank black screen
*/}
{powered && !initialConnect ? (
isRetryingConnection ? (
this.renderRetryState()
) : (
<React.Fragment />
)}

{powered && activeVnc && token && region && (
<div
className={classes.container}
style={!initialConnect ? { display: 'none' } : {}}
>
<VncDisplay
url={`${getLishSchemeAndHostname(region)}:8080/${token}`}
onUpdateState={this.onUpdateVNCState}
onResize={this.canvasResize}
/>
</div>
)}
</div>
<CircleProgress noInner />
)
) : (
<React.Fragment />
)}

{powered && activeVnc && token && region && (
<div
className={classes.container}
style={!initialConnect ? { display: 'none' } : {}}
>
<VncDisplay
url={`${getLishSchemeAndHostname(region)}:8080/${token}`}
onUpdateState={this.onUpdateVNCState}
onResize={this.canvasResize}
/>
</div>
)}
</div>
);
}
Expand Down
138 changes: 39 additions & 99 deletions packages/manager/src/features/Lish/Lish.tsx
View file
Open in desktop
Expand Up @@ -4,52 +4,34 @@ import {
Linode
} from '@linode/api-v4/lib/linodes';
import * as React from 'react';
import {
matchPath,
Route,
RouteComponentProps,
Switch,
withRouter
} from 'react-router-dom';
import { matchPath, RouteComponentProps, withRouter } from 'react-router-dom';
import CircleProgress from 'src/components/CircleProgress';
import TabPanel from 'src/components/core/ReachTabPanel';
import TabPanels from 'src/components/core/ReachTabPanels';
import {
createStyles,
Theme,
withStyles,
WithStyles
} from 'src/components/core/styles';
import Tab from 'src/components/core/Tab';
import Tabs from 'src/components/core/Tabs';
import Tabs from 'src/components/core/ReachTabs';
import TabLinkList from 'src/components/TabLinkList';
import Typography from 'src/components/core/Typography';
import ErrorState from 'src/components/ErrorState';
import NotFound from 'src/components/NotFound';
import { convertForAria } from 'src/components/TabLink/TabLink';
import Glish from './Glish';
import Weblish from './Weblish';

type ClassNames = 'tabs' | 'tabRoot' | 'progress' | 'notFound';
type ClassNames = 'tabs' | 'progress' | 'notFound';

const AUTH_POLLING_INTERVAL = 2000;

const styles = (theme: Theme) =>
createStyles({
tabs: {
backgroundColor: theme.bg.offWhite,
backgroundColor: 'black',
margin: 0
},
tabRoot: {
margin: 0,
flexBasis: '50%',
transition: theme.transitions.create('background-color'),
'&[aria-selected="true"]': {
backgroundColor: theme.palette.primary.main,
color: 'white',
'&:hover': {
backgroundColor: theme.palette.primary.light,
color: 'white'
}
}
},
progress: {
height: 'auto'
},
Expand Down Expand Up @@ -188,53 +170,23 @@ class Lish extends React.Component<CombinedProps, State> {
});
};

handleTabChange = (
event: React.ChangeEvent<HTMLDivElement>,
value: number
) => {
const { history } = this.props;
const routeName = this.tabs[value].routeName;
history.push(`${routeName}`);
};

tabs = [
/* NB: These must correspond to the routes inside the Switch */
{ routeName: `${this.props.match.url}/weblish`, title: 'Weblish' },
{ routeName: `${this.props.match.url}/glish`, title: 'Glish' }
{
title: 'Weblish',
routeName: `${this.props.match.url}/weblish`
},
{
title: 'Glish',
routeName: `${this.props.match.url}/glish`
}
];

matches = (p: string) =>
Boolean(matchPath(p, { path: this.props.location.pathname }));

renderWeblish = () => {
const { linode, token } = this.state;
if (linode && token) {
return (
<Weblish
token={token}
linode={linode}
refreshToken={this.refreshToken}
/>
);
}
return null;
};

renderGlish = () => {
const { linode, token } = this.state;
if (linode && token) {
return (
<Glish token={token} linode={linode} refreshToken={this.refreshToken} />
);
}
return null;
};

render() {
const {
classes,
match: { path }
} = this.props;
const { classes } = this.props;
const { authenticated, loading, linode, token } = this.state;

// If the window.close() logic above fails, we render an error state as a fallback
Expand All @@ -251,49 +203,37 @@ class Lish extends React.Component<CombinedProps, State> {
);
}

const tabA11yProps = (idName: string) => {
const ariaVal = convertForAria(idName);

return {
id: `tab-${ariaVal}`,
role: 'tab',
'aria-controls': `tabpanel-${ariaVal}`
};
};

return (
<React.Fragment>
<Tabs
value={this.tabs.findIndex(tab => this.matches(tab.routeName))}
onChange={this.handleTabChange}
className={classes.tabs}
indicatorColor="primary"
textColor="primary"
scrollButtons="off"
>
{this.tabs.map(tab => (
<Tab
classes={{
root: classes.tabRoot
}}
key={tab.title}
label={tab.title}
data-qa-tab={tab.title}
{...tabA11yProps(tab.title)}
/>
))}
<Tabs className={classes.tabs}>
<TabLinkList lish tabs={this.tabs} />
<TabPanels>
<TabPanel data-qa-tab="Weblish">
{linode && token && (
<Weblish
token={token}
linode={linode}
refreshToken={this.refreshToken}
/>
)}
</TabPanel>
<TabPanel data-qa-tab="Glish">
{linode && token && (
<Glish
token={token}
linode={linode}
refreshToken={this.refreshToken}
/>
)}
</TabPanel>
))}
</TabPanels>
</Tabs>
{loading && <CircleProgress noInner className={classes.progress} />}
{/* Only show 404 component if we are missing _both_ linode and token */}
{!loading && !linode && !token && (
<NotFound className={classes.notFound} />
)}
{!loading && token && linode && (
<Switch>
<Route exact path={`${path}/weblish`} render={this.renderWeblish} />
<Route exact path={`${path}/glish`} render={this.renderGlish} />
</Switch>
)}
</React.Fragment>
);
}
Expand Down