Add multiple image compression options

[mingming-ma]

Description

Allow user to set the preferred image compression options, the settings modal of this part looks like:

Notes for the slider of Maximum image dimension option, I set the step to 16, Therefore, users will not feel the tediousness of changing the numerical values, and I think the step makes sense in terms of image dimensions.

min={16}
max={2048}
step={16}

Fixes #464

[cloudflare-pages]

Deploying chatcraft-org with    Cloudflare Pages

Latest commit:	d76e427
Status:	✅  Deploy successful!
Preview URL:	https://5528c287.console-overthinker-dev.pages.dev
Branch Preview URL:	https://mingming-compressionoptions.console-overthinker-dev.pages.dev

View logs

[Amnish04]

@mingming-ma Works well! Just left some comments for some things.

[Amnish04]

Suggested change

	compressionFactor: number = 1,
	maxSizeMB: number = 20,
	maxWidthOrHeight: number = 2048
	compressionFactor: number = getSettings().compressionFactor,
	maxSizeMB: number = getSettings().maxCompressedFileSizeMB,
	maxWidthOrHeight: number = getSettings().maxImageDimension

Instead of hard-coding default values, shouldn't we use the current settings values?
This would eliminate the need to pass settings.xyz explicitly wherever you are currently using these.

But you might also want to keep it decoupled from settings state, as you are doing right now, so its a personal preference I guess.

Also, shouldn't we rather use an options object instead of flat args. For cases where I want to pass only one option, currently I will be forced to pass other args as well.

[mingming-ma]

The options object idea is great! Let me fix that.
For the hard-coding default values, I prefer to keep it decoupled from settings as it's in the util

[Amnish04]

I noticed that you have 3 controls in just one FormControl component. I am not sure if that's valid Html.

We should probably have separate FormControl components for each slider and find a different way to show them as an "Image Compression" group (Maybe a heading tag and some separators?)

Also, there should be separate FormHelperTexts for each slider in my opinion

[mingming-ma]

I understand your concerns. I tried to use heading and some div hack but not have good alignment with existing settings, after some research, I think here we can use fieldset to make sure it belong to same group and should be good to generate valid Html. and I tested in chrome, firefox and safari, and rendering ok and works well in those, should we deliver this approach ?

[Amnish04]

@mingming-ma Looks way better now!

Just one more concern, can we have each slider in its own FormControl with a separate FormHelperText only focusing on that particular control?

Something like this

<FormControl as="fieldset">
  <FormControl as="legend">Image Compression</FormControl>
  <Stack spacing={5} ps={3} pt={2}>
    <FormControl>
      <FormLabel>
        Maximum file size after compression: {settings.maxCompressedFileSizeMB} (MB)
      </FormLabel>
      <Slider
        id="max-compressed-file-size"
        value={settings.maxCompressedFileSizeMB}
        onChange={(value) =>
          setSettings({ ...settings, maxCompressedFileSizeMB: value })
        }
        min={1}
        max={20}
        step={1}
      >
        <SliderTrack>
          <SliderFilledTrack />
        </SliderTrack>
        <SliderThumb />
      </Slider>
      <FormErrorMessage>
        Maximum file size must be between 1 and 20 MB.
      </FormErrorMessage>

      <FormHelperText>...</FormHelperText>
    </FormControl>

    <FormControl>
      <FormLabel>
        Maximum image dimension: {settings.maxImageDimension} (px)
      </FormLabel>
      <Slider
        id="max-image-dimension"
        value={settings.maxImageDimension}
        onChange={(value) => setSettings({ ...settings, maxImageDimension: value })}
        min={16}
        max={2048}
        step={16}
      >
        <SliderTrack>
          <SliderFilledTrack />
        </SliderTrack>
        <SliderThumb />
        <FormErrorMessage>
          Maximum Image dimension must be between 16 and 2048
        </FormErrorMessage>
      </Slider>

      <FormHelperText>...</FormHelperText>
    </FormControl>

    <FormControl>
      <FormLabel>Compression factor: {settings.compressionFactor}</FormLabel>
      <Slider
        id="compression-factor"
        value={settings.compressionFactor}
        onChange={(value) => setSettings({ ...settings, compressionFactor: value })}
        min={0.1}
        max={1}
        step={0.1}
      >
        <SliderTrack>
          <SliderFilledTrack />
        </SliderTrack>
        <SliderThumb />
        <FormErrorMessage>
          Compression factor must be between 0.1 and 1.0
        </FormErrorMessage>
      </Slider>

      <FormHelperText>...</FormHelperText>
    </FormControl>
  </Stack>
</FormControl>

[mingming-ma]

@Amnish04

So I guess we dont need the final message in the bottom, Any suggestions?

Update: I missed the "attached" info, let me update the helper messages

[Amnish04]

@mingming-ma Looks good now. Thanks for the changes!

[Amnish04]

So I guess we dont need the final message in the bottom, Any suggestions?

Yes, I don't think we need that as we have separate hints for each control

[mingming-ma]

@Amnish04 Thanks a lot for the feedback!

[humphd]

I don't love how much length this adds to the modal, and wonder if (in a follow-up) we should make this an advanced feature you can expand (accordion style?). I also don't know what the final value is doing (the explanation needs more clarity about what each value will do). Again, that can happen in a follow up.

[Amnish04]

I don't love how much length this adds to the modal, and wonder if (in a follow-up) we should make this an advanced feature you can expand (accordion style?). I also don't know what the final value is doing (the explanation needs more clarity about what each value will do). Again, that can happen in a follow up.

@humphd Just filed a follow up for that