Feature/display attribute for elements other than <Box>

[Kushagra-ag]

This is a good to have feature as there's a workaround for this issue at present.

Usecase - Developer wants to hide the below link component on mobile.

<Link display={{ base: 'none', l: 'block' }} > I am link </Link>

This is not possible currently as Link component doesn't support display attribute.

Workaround -

<Box display={{ base: 'none', l: 'block' }} >
  <Link> I am link </Link>
</Box>

[saurabhdaware]

So we don't support display property because it can be misused (E.g. someone might just make Badge as a display block which will change how the Badge looks)

One way could be to support just none and default on display but I feel then we'll deviate from actual possible CSS values. We can take a call if more people request for this.

For now, if you want, you can also create maybe <Mobile /> and <Desktop /> (or any other name) type of wrapper components in your repo if you feel writing display object again and again is too verbose.

<Mobile>
  <Button>Mobile Button</Button>
</Mobile>
<Desktop>
  <Button>Desktop Button</Button>
</Desktop>

[divyanshu013]

Agreed, we brainstormed this a lot on layout RFC and decided to not support display props intentionally on the presentational components (because they can mess the component's layout).

[saurabhdaware]

I think we can close this for now. If we get more people requesting this, we can take a look again.

[Kushagra-ag]

One caveat here is unnecessary DOM element that is added in the workaround suggested

[divyanshu013]

Yes, that is an inconvenience. Unfortunately that is the best way currently to prevent components misuse. Open to other suggestions that can check all the boxes in the RFC.

[saurabhdaware]

Reopening this because we got another request https://razorpay.slack.com/archives/CMQ3RBHEU/p1685967672996579?thread_ts=1685526177.654729&cid=CMQ3RBHEU.

Also came up several times while mentioning inconvenience of creating Box wrappers. Can keep this open to make sure we dont miss out on some opinions.

[cseas]

someone might just make Badge as a display block which will change how the Badge looks

Pretty sure this is impossible. The devs are only trying to match what the designers have built, not ship broken designs on purpose. In hopes of preventing impossible cynical scenarios, we're blocking devs from many genuine use-cases that are a no-brainer for Blade to support from the beginning. [Example]

We were supposed to complete Hero section of Payment Pages last week but keep getting into discussion loops with Blade team. Please expedite the decision process to clear the blockers mentioned here: https://github.com/razorpay/frontend-website/pull/2137#issue-1731016789 // @nashcheez @kuldeepak-razor @snehasish27

[saurabhdaware]

We were supposed to complete Hero section of Payment Pages last week but keep getting into discussion loops with Blade team. Please expedite the decision process to clear the blockers mentioned here: https://github.com/razorpay/frontend-website/pull/2137#issue-1731016789 // @nashcheez @kuldeepak-razor @snehasish27

None of these are blockers right? they can be solved by creating Box wrappers so product releases shouldn't be blocked on this. I understand its DX inconvenience but lets separate out DX inconvenience with blockers.

About the issue itself, we are discussing it but atleast from blade's end, I wouldn't mark this as a blocker for consumer.

[cseas]

This design is just impossible with Blade in the current state. Not seeing any workaround for this. This would've been a non-issue with any other UI library.

[saurabhdaware]

You can either wrap it in display flex Box like in React Native or ignore the TS error and add span with that color. https://codesandbox.io/s/title-with-colors-yx5lvf?file=/src/App.tsx

Understandable that its a workaround and not a solution but give us some time to discuss and fix this

[cseas]

First suggestion will not work, we need the colored text to be inline, part of the same sentence, not a separate block. Making this responsive will be chaotic.

Second suggestion is clearly brute forcing our way through ignoring Blade API which goes against all guidelines. I'll go ahead and add this to get unblocked but since this is clear violation of the API, hoping this code won't break in future.

[saurabhdaware]

Conclusion

So we discussed this issue internally and all of us came to same conclusion that although there are chances of few components breaking, display seems like a common prop on component especially for hiding and showing component based on screen size with -

<Link display={{ base: 'none', m: 'inline' }}>Desktop Link!!</Link> 

1. So in this case, because of common usage, the DX convenience would take higher priority over chance of some component breaking.
2. Semantically, we hide / show certain components (like Link, Button) and not entire containers. So makes sense to be able to hide / show them directly without toggling the visibility of entire container (Box).
3. In most cases, passing incorrect display will completely break component rather than make it look different so consumer engineers won't ship it in the first place (I have elaborated more about why we put constraints in general and overall philosophy here - https://github.com/razorpay/blade/issues/1214#issuecomment-1582191138)

Conclusion: display will be supported on all major components as part of styled-props. We will take this up in future sprint.

[anuraghazra]

Conclusion: display will be supported on all major components as part of styled-props. We will take this up in future sprint.

FYI, Will need to add display property on icons too.

By default SVGs are inline elements thus they will accommodate extra space at the bottom for the dangling texts. This creates an issue where vertically aligning the icon visually with another element will become hard (with flex etc), thus we need <Icon display="block" />

https://codesandbox.io/s/optimistic-cloud-g687dd?file=/App.tsx

[chaitanyadeorukhkar]

Another consumer ask for this: https://razorpay.slack.com/archives/CMQ3RBHEU/p1688466727053259

Needs to be prioritized (Will get this done)