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

[RFC] Grid #535

Closed
connor-baer opened this issue Jan 29, 2020 · 11 comments
Closed

[RFC] Grid #535

connor-baer opened this issue Jan 29, 2020 · 11 comments
Labels
feature A new feature or enhancement 📝 RFC Request for comment stale

Comments

@connor-baer
Copy link
Member

connor-baer commented Jan 29, 2020

Components to amend

Grid, Row, Col

Context

Circuit UI currently provides a float-based grid system. This is mostly because SumUp's dashboard is still supporting very old browsers (i.e., webviews on old Android phones) with bad flexbox and no CSS grid support. Users of Circuit UI aren't all happy with this and provided some feedback:

  • "It would be nice to add more helpers to the Row and Col components, for aligning elements and to have more flexibility"
  • The grid components add several layers of divs that pollute the JSX and DOM.
  • Nested grids are not possible since the gutters don't match up

Goals

  • Gutters: These are arguably the most important part of a grid system since they determine alignment. They need to be consistent even when nested.
  • Placement: Users need to be able to place items anywhere inside the grid system and align them to any side.
  • Responsive: Gutters and placement need to be adjustable across viewports.

Proposals

1. No grid / BYO

CSS Grid is now supported by 93.5% of globally used browsers. It covers even the most advanced use cases (except nested subgrids). Plus there are a plethora of third-party React grid libraries to choose from. So do we still need to provide a grid system with Circuit UI?

The only thing we need to provide is consistent values for the gutter in the theme (see also #534).

2. Improve the grid

We could add additional features to the current grid components, e.g. to support nested grids. Some users also suggested to switch to flexbox and while flexbox is arguably more powerful than floats, it's still only meant for one-dimensional layouts (versus CSS Grid's two dimensions).

@connor-baer connor-baer added feature A new feature or enhancement 📝 RFC Request for comment labels Jan 29, 2020
@connor-baer connor-baer added this to the v3.0 milestone Jan 29, 2020
@connor-baer connor-baer mentioned this issue Jan 29, 2020
16 tasks
@larimaza
Copy link
Contributor

larimaza commented Feb 3, 2020

  1. Props for the use of "plethora", very fancy;
  2. I vote for the "No grid" approach, because I imagine any implementation we create will be mainly a pure abstraction of the existing grid API without additional features that are specific to our use cases. If that's not the case and we in fact need to add more features, we'd need to list these first to make sure we have enough reasons :)

@fernandofleury
Copy link
Contributor

I think we should move to the no grid approach. My biggest concern is that gutter/col definition exists (and should be followed, since it it's part of our visual language) within our design system. By removing the grid and letting the consumer decide how to arrange things, it will probably mean we will end up with different spacings across different domains.

@fernandofleury
Copy link
Contributor

And we can't also discard the convenience of grid as utility vs redoing it for scratch over and over. If we don't provide a built in grid, we could maybe nudge into the right direction like suggesting libraries (like https://rebassjs.org/reflexbox/) and configuration for it to match our gutter/col usage.

@connor-baer
Copy link
Member Author

Alright, there seems to be a consensus to drop support for a grid in Circuit UI. What does that mean for the existing Grid components? I'd like to discourage their use, but do we fully deprecate them or keep them around for legacy reasons?

@robinmetral
Copy link
Contributor

robinmetral commented Feb 7, 2020

I would deprecate and link to documentation for migrating to any other grid system we choose. We could write it in a README in components/Grid for example. WDYT?

@fernandofleury
Copy link
Contributor

fernandofleury commented Feb 7, 2020

I think we need to discuss this in further details before we go for deprecation. The majority of our web context is still using the Grid.

What I propose is:

  • Let's discuss new ways of working with a grid that is compliant to our design-system guidelines, and considers different levels of cross-browser support;
  • Run a POC replacing the current Grid usage;
  • Wrap up the POC & publish new documentation reflecting the usage;
  • Deprecate current Grid component

Let's be careful and remember that the design system is meant to power up our developers in their current scenarios. Not provide an experience that most of them can't comply to.

@robinmetral
Copy link
Contributor

Naturally I wasn't suggesting a deprecation without a solid proof of concept + migration guide. Looks like we're on the same page 👌

@felixjung
Copy link
Collaborator

Hey, I'm late to the party. :D Generally I feel we don't need a dependency for the Grid. At this point, I think even the Dashboard would be fine with Flexbox (and maybe the Grid?). Could well be that we bump the build targets for iOS and Android early next year.

I still feel we should

  • get clear input from design with respect to what types of grids they want to support and
  • implement those grids in Circuit UI.

We want to make sure designs and implementations use the grids that are part of the design system and we want consistency.

@connor-baer connor-baer mentioned this issue Dec 9, 2020
49 tasks
@felixjung
Copy link
Collaborator

Here's an adaptation of the Circuit UI Grid using CSS Grid. The API should be pretty much identical and so should the spacing/gutter be.

@connor-baer connor-baer modified the milestones: v3.0, v4.0 Apr 12, 2021
@connor-baer connor-baer modified the milestones: v4.0, v6.0 Jan 7, 2022
robinmetral pushed a commit that referenced this issue Apr 21, 2022
We're still using a float-based grid but it's not because of browser support anymore. It's because we haven't updated it yet. See #535
robinmetral pushed a commit that referenced this issue Apr 22, 2022
We're still using a float-based grid but it's not because of browser support anymore. It's because we haven't updated it yet. See #535
robinmetral pushed a commit that referenced this issue Apr 26, 2022
* Remove workarounds for legacy browsers

* Add draft migration guide entry and remove polyfill docs

* Remove mention of browser support from Grid docs

We're still using a float-based grid but it's not because of browser support anymore. It's because we haven't updated it yet. See #535

* Start using onChange instead of onClick in RadioButton and Checkbox

* Remove unused shared prop types

* Remove unused script

The old clean script was pointing to an old path

* Reintroduce clean script but fixed

* Remove unnecessary skipLibCheck from tsconfigs

* Deduplicate

* Add draft docs

* Remove unused imports in SB docs

* Compile Circuit UI to es2018

* Update docs

* Transpile to es2017

* Update docs and add draft contributor docs for browser support

* Do not include all @types

* Remove unused imports

* Fix storybook warning

* Update browser support for contributors docs

* Add changeset
@connor-baer connor-baer added this to the v6.0 milestone Jun 1, 2022
@robinmetral robinmetral removed this from the v6.0 milestone Jul 21, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Add a comment explaining why the issue is still relevant to prevent it from being closed.

@connor-baer
Copy link
Member Author

The grid components will be marked as legacy in Circuit UI v7. We recommend using the native CSS Grid instead.

We might explore patterns for column and gutter widths in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature or enhancement 📝 RFC Request for comment stale
Projects
None yet
Development

No branches or pull requests

5 participants