treetracker-web-map-client icon indicating copy to clipboard operation
treetracker-web-map-client copied to clipboard

CWM defined spacing / sizes

Open RubenSmn opened this issue 2 years ago • 9 comments
trafficstars

CWM defined spacing

Idea

The ability to adapt to the design of this issue. Sizes / spacing can be used for all kind of things like, padding and margin but even border-radius. To be able to customize this will introduce a whole new level of customization we can provide for our partners but also for ourselves.

How to get there

The MUI library has a system for spacing. This is very flexible and you can get any spacing you want. You can customize this by using the createTheme function.

This property is currently not customizable due to some serialization and we set a default size of 4, see here for the code.

As of now I see 2 ways of approaching the challenge to customize the spacing.

1. Customize the spacing prop with the CWM

This will allow us to use the predefined MUI theme.spacing. The spacing prop is a single value. In the example below the value is 8

<Box
  sx={{
    m: (theme) => theme.spacing(1, "auto"), // '8px auto'
  }}
></Box>

This works fine but the ability to customize is limited since we can only change 1 value.

2. Add custom sizing properies xs sm md lg ... etc to the theme

This will give us more control over the specific spacing / sizes. We can add a sizes prop to the theme as shown below. For the default sizes I think we should take a look at Tailwind CSS spacing.

// in px units
const ourCustomTheme = {
  sizes: {
    xs: 2,
    sm: 4,
    md: 8,
    lg: 16,
  },
  palette: {
    // ...
  },
  // the rest of the theme
};

Usage

<Box
  sx={{
    m: (theme) => theme.sizes.md, // 8px
  }}
></Box>

Drawback

The drawback for both of these solutions is that all the margin padding etc props in the web-map-client need to be rewritten to use the new size system to have effect. The is necessary with whatever option you go.

RubenSmn avatar Mar 13 '23 20:03 RubenSmn

@dadiorchen @jeremydthomas @andrewsu31098 @keyy123 would like to get your opinions on this.

RubenSmn avatar Mar 13 '23 20:03 RubenSmn

@RubenSmn, From a design standpoint I think building these systems will accelerate frontend development for the entire project in the long run but we definitely need to visualize the impact this system has on each view of the web-map-client then tickets creation.

TLDR: I think it's a good idea just need to see what changed and make tickets to assign

keyy123 avatar Mar 13 '23 21:03 keyy123

Agree with keyy123.

jeremydthomas avatar Mar 13 '23 21:03 jeremydthomas

I agree. Either implementation looks good. And old code can be refactored on a case-by-case basis. MUI refactors make really good first issues too :)

andrewsu31098 avatar Mar 14 '23 03:03 andrewsu31098

I long for our visual test in place, or as a compromised solution, a screenshot comparison. @RubenSmn thanks for the good proposal, questions from me:

  1. For option 2, is the sizes a built-in or conversion from MUI? or we define it as wish?
  2. For option 1, say, the theme for Greenstand set spacing to 4px (current situation), and to fit the design of the wallet UI in the Figma link, we suppose to set the value to 2px to get a more compact spacing style, is this how it works, right? Seems option 2 is not so good, we need to change the size in every detailed place, right?
  3. For option 2, is it possible we can set the size in some special cases like an override solution? <Box sx={{ m: themeName === 'wallet': '2px' : theme.spacing(1) }} > , maybe this is too much, and it should be a rare case.

dadiorchen avatar Mar 14 '23 08:03 dadiorchen

@keyy123 @andrewsu31098 @jeremydthomas @dadiorchen thank for the feedback and insights. As for the questions:

For option 2, is the sizes a built-in or conversion from MUI? or we define it as wish?

  1. We can define the sizes as we wish.

For option 1, say, the theme for Greenstand set spacing to 4px (current situation), and to fit the design of the wallet UI in the Figma link, we suppose to set the value to 2px to get a more compact spacing style, is this how it works, right? Seems option 2 is not so good, we need to change the size in every detailed place, right?

  1. Yes correct, when using option 1 everything will get 2px smaller / half sized. The downside here is that everything changes. There might be a place where you still want let's say 64px of margin, this will now be 32px and you'll need to go inside the actual code to set the spacing to a larger value.

For option 2, is it possible we can set the size in some special cases like an override solution? <Box sx={{ m: themeName === 'wallet': '2px' : theme.spacing(1) }} > , maybe this is too much, and it should be a rare case.

  1. I guess this is possible, I think for these cases we can simply send in a whole new theme object with the relevant sizes and colors etc. The wallet web app is a totally different web app, I think we can make it possible to load that web app in the cwm editor and customize it the same as we're currently doing with the web client app.

RubenSmn avatar Mar 20 '23 14:03 RubenSmn

Prerequisite #1604

RubenSmn avatar May 25 '23 20:05 RubenSmn

@RubenSmn I'm thinking if we just use current mechanism to modify the theme to try to adapt the style in the wallet design, how far can we go, I mean, how close we can make it to looks like that design? Maybe for the customized map, the bar couldn't be too high.

dadiorchen avatar May 27 '23 06:05 dadiorchen

@dadiorchen we could go pretty far. But I've already got the new system in place to be able to handle new inputs and improve the editor #1619

RubenSmn avatar May 30 '23 20:05 RubenSmn