Skip to content

Commit

Permalink
[dagit] Clean up my todos (#7810)
Browse files Browse the repository at this point in the history
### Summary & Motivation

Clean up some todos I had left for myself. The Blueprint button classes shouldn't be used anymore, and the `material-icons` class should not be relevant at this point.

I also cleaned up the button to download compute logs, since that had a bp3-button class and an old icon on it.

<img width="362" alt="Screen Shot 2022-05-09 at 2 47 39 PM" src="https://user-images.githubusercontent.com/2823852/167486465-e65ec775-29a0-417f-8194-412bd3f47965.png">


### How I Tested These Changes

Buildkite. View Run compute logs in Dagit, verify rendering and behavior of download button.
  • Loading branch information
hellendag committed May 10, 2022
1 parent 0540339 commit 14947c2
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 56 deletions.
15 changes: 0 additions & 15 deletions js_modules/dagit/packages/core/.storybook/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,6 @@ const GlobalStyle = createGlobalStyle`
font-family: ${FontFamily.monospace};
font-size: 16px;
}
.material-icons {
display: block;
}
/* todo dish: Remove these when we have buttons updated. */
.bp3-button .material-icons {
position: relative;
top: 1px;
}
.bp3-button:disabled .material-icons {
color: ${Colors.Gray300}
}
`;

// Global decorator to apply the styles to all stories
Expand Down
15 changes: 0 additions & 15 deletions js_modules/dagit/packages/core/src/app/AppProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,6 @@ const GlobalStyle = createGlobalStyle`
font-family: ${FontFamily.monospace};
font-size: 16px;
}
.material-icons {
display: block;
}
/* todo dish: Remove these when we have buttons updated. */
.bp3-button .material-icons {
position: relative;
top: 1px;
}
.bp3-button:disabled .material-icons {
color: ${Colors.Gray300}
}
`;

export interface AppProviderProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ import {ConfigEditorRunConfigSchemaFragment} from './types/ConfigEditorRunConfig
interface ConfigEditorProps {
configCode: string;
readOnly: boolean;
// todo dish: Remove this prop
showWhitespace?: boolean;
runConfigSchema?: ConfigEditorRunConfigSchemaFragment;

checkConfig: YamlModeValidateFunction;
Expand Down
44 changes: 35 additions & 9 deletions js_modules/dagit/packages/core/src/runs/LogsToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import {
Tab,
Tabs,
Tag,
IconWrapper,
Colors,
Tooltip,
} from '@dagster-io/ui';
import * as React from 'react';
import styled from 'styled-components/macro';
Expand Down Expand Up @@ -186,7 +189,7 @@ const ComputeLogToolbar = ({
) : null}
</Group>
{isValidStepSelection ? (
<Group direction="row" spacing={12} alignItems="center">
<Box flex={{direction: 'row', alignItems: 'center', gap: 12}}>
{computeLogKey && logCaptureSteps[computeLogKey] ? (
resolveState(metadata, logCaptureSteps[computeLogKey]) === IStepState.RUNNING ? (
<Spinner purpose="body-text" />
Expand All @@ -195,24 +198,47 @@ const ComputeLogToolbar = ({
)
) : null}
{computeLogUrl ? (
<a
aria-label="Download link"
className="bp3-button bp3-minimal bp3-icon-download"
href={computeLogUrl}
title={
<Tooltip
placement="top-end"
content={
computeLogKey && logCaptureSteps[computeLogKey]?.stepKeys.length === 1
? `Download ${logCaptureSteps[computeLogKey]?.stepKeys[0]} compute logs`
: `Download compute logs`
}
download
></a>
>
<DownloadLink aria-label="Download link" href={computeLogUrl} download>
<Icon name="download_for_offline" color={Colors.Gray600} />
</DownloadLink>
</Tooltip>
) : null}
</Group>
</Box>
) : null}
</Box>
);
};

const DownloadLink = styled.a`
border-radius: 4px;
display: block;
text-decoration: none;
${IconWrapper} {
transition: background-color 100ms linear;
}
:hover ${IconWrapper} {
background-color: ${Colors.Gray800};
}
:active ${IconWrapper}, :focus ${IconWrapper} {
background-color: ${Colors.Dark};
}
:focus {
outline: none;
}
`;

const StructuredLogToolbar = ({
filter,
onSetFilter,
Expand Down
15 changes: 0 additions & 15 deletions js_modules/dagit/packages/ui/.storybook/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,6 @@ const GlobalStyle = createGlobalStyle`
font-family: ${FontFamily.monospace};
font-size: 16px;
}
.material-icons {
display: block;
}
/* todo dish: Remove these when we have buttons updated. */
.bp3-button .material-icons {
position: relative;
top: 1px;
}
.bp3-button:disabled .material-icons {
color: ${Colors.Gray300}
}
`;

// Global decorator to apply the styles to all stories
Expand Down

1 comment on commit 14947c2

@vercel
Copy link

@vercel vercel bot commented on 14947c2 May 10, 2022

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

dagit-storybook – ./js_modules/dagit/packages/ui

dagit-storybook-elementl.vercel.app
dagit-storybook.vercel.app
dagit-storybook-git-master-elementl.vercel.app

Please sign in to comment.