search

processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.38k stars 1.32k forks source link

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

Closed lindapaiste closed 9 months ago

lindapaiste commented 9 months ago

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.

https://github.com/processing/p5.js-web-editor/blob/e77bc6ac3e5fd499a12821fef8ecac91a172fa3a/client/common/Button.jsx#L185-L189

https://github.com/processing/p5.js-web-editor/blob/e77bc6ac3e5fd499a12821fef8ecac91a172fa3a/client/common/Button.jsx#L83-L108

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. https://github.com/processing/p5.js-web-editor/blob/e77bc6ac3e5fd499a12821fef8ecac91a172fa3a/client/common/Button.jsx#L85

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.

skushagra9 commented 9 months ago

Can I work on this issue @lindapaiste

lindapaiste commented 9 months ago

Can I work on this issue @lindapaiste

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