Progressively add descriptions for codec settings #1366
Conversation
There was a problem hiding this comment.
This is an amazing quality-of-life improvement for users. Really nice work. This is a really good start.
One thing we should talk about that the text in title is effectively inaccessible to mobile users, but this is already better than having no description at all.
Maybe as a follow-up we can move to custom tooltip so that we can control when and how it appears on mobile and maybe even do rich-text formatting and links.
I also have not review the descriptions themselves just yet.
| function wrapText(text: string, line_width: number = 60): string { | ||
| const lines = text.split('\n'); | ||
| let balancedText = ''; | ||
| for (const line of lines) { | ||
| balancedText += '\n'; | ||
| const words = line.split(' '); | ||
| let runningLength = 0; | ||
| for (const word of words) { | ||
| if (runningLength + word.length + 1 > line_width) { | ||
| balancedText += '\n' + word + ' '; | ||
| runningLength = word.length; | ||
| } else { | ||
| runningLength += word.length + 1; | ||
| balancedText += word + ' '; | ||
| } | ||
| } | ||
| } | ||
| return balancedText.trim(); | ||
| } |
There was a problem hiding this comment.
This function is for the UI and should probably in src/client/lazy-app/util or something. Rather than applying the text transform here, the UI should probably do it on render.
There was a problem hiding this comment.
I also think we can simplify this. The algorithm here should probably be something like
- Split text into arrays of lines
- For each line:
- While current output line is shorter than
limit- Add the next word from the current line to the current output line
- Emit current output line
- Set output line to empty string.
- While current output line is shorter than
(This could be a great use-case for a generator)
There was a problem hiding this comment.
Ahh you're right, it would be better suited for the utilities file. I'll make a generator version as well.
| quality: | ||
| 'Trade-off between compressed file size and image quality\nHigher quality means larger file sizes', |
There was a problem hiding this comment.
If you use template literals and dedent, we can make this more readable:
| quality: | |
| 'Trade-off between compressed file size and image quality\nHigher quality means larger file sizes', | |
| quality: dedent(` | |
| Trade-off between compressed file size and image quality. | |
| Higher quality means larger file sizes | |
| `), |
There was a problem hiding this comment.
How didn't I know about dedent till now, it's exactly what I wanted LOL. I'll use it now.
|
|
||
| // Until we make a better way of displaying information (like with modals), displaying a simple 'title' attribute should suffice for each codec parameter | ||
| type mozjpeg_param = keyof typeof defaultOptions; | ||
| const paramDescriptions: Record<mozjpeg_param, string> = { |
There was a problem hiding this comment.
With the logic above removed, this file can be a singular default export of this dictionary.
| <option | ||
| value="0" | ||
| title={ | ||
| 'Standard Quant Tables given in Annex K of the CCITT paper' |
There was a problem hiding this comment.
Any reason why you put some descriptions in the meta file and some are inline? I think it’d be nice to move these into the meta file as well, so all descriptions for a codec are in one place.
There was a problem hiding this comment.
I wanna give descriptions for params and their options (like ImageMagick for quant_table). Problem is, for intellisense to do some nice autocompletion for the getParameterDescription arguments, I'll need an interface containing all params and their options, which ends up looking very verbose IMO.
So I went with using getParameterDescription for the params, and inlining the descriptions for the options.
I'll be refactoring all of this once I finish making a custom modal component (better explanations with better UI). For now I'll try to put all descriptions in the meta file; it'll definitely look cleaner once the modal is ready.
|
With the Modal PR this becomes pretty much obsolete |
Every codec has a ton of settings to mess around with, but the average user won't know what half of them mean. I'm adding small, slightly technical descriptions for each parameter in their respective component's
titleattribute so that users can hover over it and read about its purpose.All changes are progressive, so descriptions can be added on an as-needed basis.
Browser JPEGBrowser PNG