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.

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

Use transient props for all styled components #3049

Merged
merged 8 commits into from
Nov 3, 2023
Merged
24 changes: 12 additions & 12 deletions extensions/ql-vscode/src/view/common/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,46 +3,46 @@ import { ReactNode } from "react";
import { styled } from "styled-components";

type ContainerProps = {
type: "warning" | "error";
inverse?: boolean;
$type: "warning" | "error";
$inverse?: boolean;
};

const getBackgroundColor = ({ type, inverse }: ContainerProps): string => {
if (!inverse) {
const getBackgroundColor = ({ $type, $inverse }: ContainerProps): string => {
if (!$inverse) {
return "var(--vscode-notifications-background)";
}

switch (type) {
switch ($type) {
case "warning":
return "var(--vscode-editorWarning-foreground)";
case "error":
return "var(--vscode-debugExceptionWidget-border)";
}
};

const getTextColor = ({ type, inverse }: ContainerProps): string => {
if (!inverse) {
const getTextColor = ({ $type, $inverse }: ContainerProps): string => {
if (!$inverse) {
return "var(--vscode-editor-foreground)";
}

switch (type) {
switch ($type) {
case "warning":
return "var(--vscode-editor-background)";
case "error":
return "var(--vscode-list-activeSelectionForeground)";
}
};

const getBorderColor = ({ type }: ContainerProps): string => {
switch (type) {
const getBorderColor = ({ $type }: ContainerProps): string => {
switch ($type) {
case "warning":
return "var(--vscode-editorWarning-foreground)";
case "error":
return "var(--vscode-editorError-foreground)";
}
};

const getTypeText = (type: ContainerProps["type"]): string => {
const getTypeText = (type: ContainerProps["$type"]): string => {
switch (type) {
case "warning":
return "Warning";
Expand Down Expand Up @@ -108,7 +108,7 @@ export const Alert = ({
role,
}: Props) => {
return (
<Container type={type} inverse={inverse} role={role}>
<Container $type={type} $inverse={inverse} role={role}>
<Title>
{getTypeText(type)}: {title}
</Title>
Expand Down
2 changes: 1 addition & 1 deletion extensions/ql-vscode/src/view/common/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { styled } from "styled-components";

const DISABLED_VALUE = "-";

const StyledDropdown = styled.select<{ disabled?: boolean }>`
const StyledDropdown = styled.select`
width: 100%;
height: calc(var(--input-height) * 1px);
background: var(--vscode-dropdown-background);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ const MessageText = styled.div`
`;

type CodeSnippetMessageContainerProps = {
severity: ResultSeverity;
$severity: ResultSeverity;
};

const CodeSnippetMessageContainer = styled.div<CodeSnippetMessageContainerProps>`
border-color: var(--vscode-editor-snippetFinalTabstopHighlightBorder);
border-width: 0.1em;
border-style: solid;
border-left-color: ${(props) => getSeverityColor(props.severity)};
border-left-color: ${(props) => getSeverityColor(props.$severity)};
border-left-width: 0.3em;
padding-top: 1em;
padding-bottom: 1em;
Expand All @@ -58,7 +58,7 @@ export const CodeSnippetMessage = ({
children,
}: CodeSnippetMessageProps) => {
return (
<CodeSnippetMessageContainer severity={severity}>
<CodeSnippetMessageContainer $severity={severity}>
<MessageText>
{message.tokens.map((token, index) => {
switch (token.t) {
Expand Down Expand Up @@ -86,7 +86,7 @@ export const CodeSnippetMessage = ({
})}
{children && (
<>
<VerticalSpace size={2} />
<VerticalSpace $size={2} />
{children}
</>
)}
Expand Down
4 changes: 2 additions & 2 deletions extensions/ql-vscode/src/view/common/HorizontalSpace.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { styled } from "styled-components";

export const HorizontalSpace = styled.div<{ size: 1 | 2 | 3 }>`
export const HorizontalSpace = styled.div<{ $size: 1 | 2 | 3 }>`
flex: 0 0 auto;
display: inline-block;
width: ${(props) => 0.2 * props.size}em;
width: ${(props) => 0.2 * props.$size}em;
`;
6 changes: 3 additions & 3 deletions extensions/ql-vscode/src/view/common/TextButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { styled } from "styled-components";

type Size = "x-small" | "small" | "medium" | "large" | "x-large";

const StyledButton = styled.button<{ size: Size }>`
const StyledButton = styled.button<{ $size: Size }>`
background: none;
color: var(--vscode-textLink-foreground);
border: none;
cursor: pointer;
font-size: ${(props) => props.size ?? "1em"};
font-size: ${(props) => props.$size ?? "1em"};
padding: 0;
`;

Expand All @@ -26,7 +26,7 @@ const TextButton = ({
children: React.ReactNode;
}) => (
<StyledButton
size={size}
$size={size}
onClick={onClick}
className={className}
title={title}
Expand Down
4 changes: 2 additions & 2 deletions extensions/ql-vscode/src/view/common/VerticalSpace.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { styled } from "styled-components";

export const VerticalSpace = styled.div<{ size: 1 | 2 | 3 }>`
export const VerticalSpace = styled.div<{ $size: 1 | 2 | 3 }>`
flex: 0 0 auto;
height: ${(props) => 0.5 * props.size}em;
height: ${(props) => 0.5 * props.$size}em;
`;
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ export const DataFlowPaths = ({

return (
<>
<VerticalSpace size={2} />
<VerticalSpace $size={2} />
<SectionTitle>{ruleDescription}</SectionTitle>
<VerticalSpace size={2} />
<VerticalSpace $size={2} />

<PathsContainer>
<PathDetailsContainer>
Expand All @@ -66,7 +66,7 @@ export const DataFlowPaths = ({
</PathDropdownContainer>
</PathsContainer>

<VerticalSpace size={2} />
<VerticalSpace $size={2} />
<CodePath
codeFlow={selectedCodeFlow}
severity={severity}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ const DependencyContainer = styled.div`
margin-bottom: 0.8rem;
`;

const StyledVSCodeTag = styled(VSCodeTag)<{ visible: boolean }>`
visibility: ${(props) => (props.visible ? "visible" : "hidden")};
const StyledVSCodeTag = styled(VSCodeTag)<{ $visible: boolean }>`
visibility: ${(props) => (props.$visible ? "visible" : "hidden")};
`;

const UnsavedTag = ({ modelingStatus }: { modelingStatus: ModelingStatus }) => (
<StyledVSCodeTag visible={modelingStatus === "unsaved"}>
<StyledVSCodeTag $visible={modelingStatus === "unsaved"}>
Unsaved
</StyledVSCodeTag>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,29 @@ function getIcon(
if (variantAnalysisStatus === VariantAnalysisStatus.Canceled) {
return (
<>
<HorizontalSpace size={2} />
<HorizontalSpace $size={2} />
<ErrorIcon label="Some analyses were stopped" />
</>
);
} else {
return (
<>
<HorizontalSpace size={2} />
<HorizontalSpace $size={2} />
<ErrorIcon label="Some analyses failed" />
</>
);
}
} else if (skippedRepositoryCount > 0) {
return (
<>
<HorizontalSpace size={2} />
<HorizontalSpace $size={2} />
<WarningIcon label="Some repositories were skipped" />
</>
);
} else if (variantAnalysisStatus === VariantAnalysisStatus.Succeeded) {
return (
<>
<HorizontalSpace size={2} />
<HorizontalSpace $size={2} />
<SuccessIcon label="Completed" />
</>
);
Expand All @@ -68,7 +68,7 @@ export const VariantAnalysisRepositoriesStats = ({
if (variantAnalysisStatus === VariantAnalysisStatus.Failed) {
return (
<>
0<HorizontalSpace size={2} />
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure the 0 was a bug. Or if we do want to remove it, do we need to also remove some whitespace before the icon?

It might be simpler regardless to leave it for this PR and address as a separate PR. What do you think?

Screenshot before:
Screenshot 2023-11-02 at 16 10 34

Screenshot after:
Screenshot 2023-11-02 at 16 10 48

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're right. This just looked like an accidental typo, but I didn't actually check whether that was the case.

In this case, I would have expected something like 0 <ErrorIcon label="Variant analysis failed" /> rather than a separate element for the spacing.

0<HorizontalSpace $size={2} />
<ErrorIcon label="Variant analysis failed" />
</>
);
Expand Down