Doist/reactist

[Box] Add `gap` as a property of the Box component

Closed this issue ยท 5 comments

๐Ÿš€ Feature request

Motivation

The Box component already supports setting various flexbox properties, so it would be beneficial to also be able to provide a gap value using the spacing system. All modern browsers include support for it. We could also potentially use this feature in the future to rewrite the Stack and Inline components to use gap instead of margins.

Since gap is just a shorthand property for row-gap and column-gap, we should also add those as properties.

Example

<>
    <Box display="flex" gap="small">
        <Placeholder width="50px" height="50px" />
        <Placeholder width="50px" height="50px" />
        <Placeholder width="50px" height="50px" />
    </Box>

    <Box display="flex" rowGap="large" columnGap="small" flexWrap="wrap">
        <Placeholder width="100px" height="100px" />
        <Placeholder width="100px" height="100px" />
        <Placeholder width="100px" height="100px" />
    </Box>
</>

Possible implementations

Implementation for this could be very similar to the current implementation of the padding property (since gap does not support negative values).

This will be great, and it will significantly simplify how we write the styles for box, stack, inline and column components.

The thing that "worries" me is that gap is still unsupported in browsers that, although "old", some people may still be using. One may think that you can always tell them to upgrade, but there are people stuck in old macOS versions that have a cap in the version of Safari that they support.

We recently had a user reporting an issue that ended up being about their browser not supporting ResizeObserver, which was added to Safari even before support for flex gap was. But unlike ResizeObserver, which can be a progressive enhancement thing that we're ok with not supporting in older browsers, and degrade the user experience while still being usable, if we relied on flex gap for our styles, it can result in a broken styling.

It'd have to be a conscious decision that may involve the product team even, not just frontend, to decide what's the minimum browsers and macOS versions that we support. Do we have such a guideline already? cc @Doist/frontend

One may think that you can always tell them to upgrade, but there are people stuck in old macOS versions that have a cap in the version of Safari that they support.

@gnapse But they could install something like Firefox or Chrome instead, right? ๐Ÿค” Or they could download the desktop client?

It'd have to be a conscious decision that may involve the product team even, not just frontend, to decide what's the minimum browsers and macOS versions that we support. Do we have such a guideline already?

IMHO, we should not waste time and energy supporting old browsers unless we have data suggesting a sizeable number of users using those old browsers.

The thing that "worries" me is that gap is still unsupported in browsers that, although "old", some people may still be using.

I don't think this prevents us from introducing gap to the Box component, but it may limit how widely we roll out the use of it for the time being. The official stance for Todoist is that it only supports the latest browsers, so technically it's fine. But I agree that it's not super user friendly, and should probably be handled with caution.

I opened an internal discussion to involve others in Doist to make a decision.

gnapse commented

This is finally happening in #739. There's already an assigned reviewer, but if any of you involved in this issue in the past want to take a look, any additional pair of eyes is welcome. The change was not trivial to achieve, especially on the Columns component, so it might be worth any extra checks.