Skip to content

Button component has a different set of styles with prop display="inline" #2821

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

Closed
lindapaiste opened this issue Jan 1, 2024 · 2 comments · Fixed by #2822
Closed

Button component has a different set of styles with prop display="inline" #2821

lindapaiste opened this issue Jan 1, 2024 · 2 comments · Fixed by #2822
Assignees
Labels
Area: Code Quality For refactoring, cleanup, or improvements to maintainability Enhancement Improvement to an existing feature

Comments

@lindapaiste
Copy link
Collaborator

Increasing Access

We want to have a useful set of basic UI components that we can use throughout the app. They should be customizable via props but the output should be predictable.

We are not using the inline version anywhere because it is not useful in it's current state. If we fix it up then it can be used for issues like #2734.

Feature enhancement details

Our Button UI component has a prop display which supports values "block" and "inline". I would expect that a <Button/> with display="inline" would look the same as the normal button, with the only difference being that it is on the same line instead of a new line. In reality it uses a completely different styled component.

let StyledComponent = StyledButton;
if (display === displays.inline) {
StyledComponent = StyledInlineButton;
}

const StyledInlineButton = styled.button`
&&& {
display: flex;
justify-content: center;
align-items: center;
text-decoration: none;
color: ${prop('primaryTextColor')};
cursor: pointer;
border: none;
line-height: 1;
svg * {
fill: ${prop('primaryTextColor')};
}
&:disabled {
cursor: not-allowed;
}
> * + * {
margin-left: ${remSize(8)};
}
}
`;

The inline version is always the same color regardless of the kind="primary" and kind="secondary" props, as the color is hard-coded.

This is what I get when using inline buttons:
Screenshot 2024-01-01 14 44 10

The wildest part is that it's not even inline! It has display: flex instead of display: inline-flex.

I would expect something more like this, which is the block style button but displayed inline:
Screenshot 2024-01-01 14 42 53

My proposal is that we should delete the StyledInlineButton styled component. Instead, we should adjust the existing StyledButton to change its display property based on the display prop. This ensures that all other styling is consistent regardless of the display prop.

@lindapaiste lindapaiste added Enhancement Improvement to an existing feature Area: Code Quality For refactoring, cleanup, or improvements to maintainability labels Jan 1, 2024
@lindapaiste lindapaiste self-assigned this Jan 1, 2024
@skushagra9
Copy link

Can I work on this issue @lindapaiste

@lindapaiste
Copy link
Collaborator Author

Can I work on this issue @lindapaiste

It's already done in #2822 (pending review).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Code Quality For refactoring, cleanup, or improvements to maintainability Enhancement Improvement to an existing feature
Projects
None yet
2 participants