Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve color widget picker and types #5948

Merged
merged 3 commits into from
Apr 5, 2024
Merged

Improve color widget picker and types #5948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c5e050
Improve ColorPickerWidget types
sneridagh Apr 5, 2024
1ffd582
Immprove types for widgets
sneridagh Apr 5, 2024
5161f5b
Changelog
sneridagh Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/types/news/5948.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Split widgets type definitions into their own interfaces so they are extendable @sneridagh
199 changes: 108 additions & 91 deletions packages/types/src/config/Widgets.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,95 +1,112 @@
export interface WidgetsConfigById {
schema: React.ComponentType;
subjects: React.ComponentType;
query: React.ComponentType;
recurrence: React.ComponentType;
remoteUrl: React.ComponentType;
id: React.ComponentType;
site_logo: React.ComponentType;
}

export interface WidgetsConfigByWidget {
textarea: React.ComponentType;
datetime: React.ComponentType;
date: React.ComponentType;
password: React.ComponentType;
file: React.ComponentType;
align: React.ComponentType;
buttons: React.ComponentType;
url: React.ComponentType;
internal_url: React.ComponentType;
email: React.ComponentType;
array: React.ComponentType;
token: React.ComponentType;
query: React.ComponentType;
query_sort_on: React.ComponentType;
querystring: React.ComponentType;
object_browser: React.ComponentType;
object: React.ComponentType;
object_list: React.ComponentType;
vocabularyterms: React.ComponentType;
image_size: React.ComponentType;
select_querystring_field: React.ComponentType;
autocomplete: React.ComponentType;
color_picker: React.ComponentType;
select: React.ComponentType;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sneridagh This doesn't seem right to me. These are not interfaces with a fixed set of keys. They are mappings which can take an arbitrary number of string: React.ComponentType entries, to register new widget types. Isn't there some way we can express that in a type without needing to list all these?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The beauty of a TS interface is that you can extend them as in:

declare module '@plone/types' {
  export interface WidgetsConfigByWidget {
    BackgroundColorWidget: React.ComponentType<any>;
  }
}

anywhere, eg. in your add-on.

https://github.com/kitconcept/kitconcept.com/blob/main/frontend/src/addons/volto-kitconcept_com/src/index.ts#L50-L53

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sneridagh Yeah but it seems ridiculous if I have to update an interface just to declare that there is a new type of widget that I'd like to register, and then also actually register it.

I think these types should be declared as Record<string, React.ComponentType<any> -- https://dmitripavlutin.com/typescript-record/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davisagli You are right, in this one makes sense, because all have the same shape (They are all Record<string, React.ComponentType), in others is not that easy, and TS is not that smart when all the records do not have the same shape, or if you have to start writing complex unions.

I'd like also to standardize and make people know and use the extending interfaces pattern, so my reasoning path went in that way too. But it's true that as much as we can prevent people from writing code, we should.

Let me try it out how well it works and I will make a PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davisagli Trying it out, I remembered why :) So it's all about autocompletion too. With a generic record you cannot have both. Also this is not allowed:

export interface WidgetsConfig = Record<WidgetsConfigKeys, React.ComponentType<any>>

And you can't do:

export interface WidgetsConfig {
  default: React.ComponentType;
  id: WidgetsConfigById;
  widget: WidgetsConfigByWidget;
  vocabulary: WidgetsConfigByVocabulary;
  factory: WidgetsConfigByFactory;
  choices: React.ComponentType;
  type: WidgetsConfigByType;
  views: WidgetsConfigViews;
  [k in string]: Record<string, React.ComponentType<any>>;
}

Since Interfaces can't declare both static and dynamic keys:

https://ts-error-translator.vercel.app/?error=IIAgtghgDlCmAmIAuBPO4IpAOwPZJHlgGMAbCAJ1hCgtzgqQEtYBnEXC8WJAC13isAdEA

Since interfaces don't work like this, so you have to fall back to types, then they are not extendable.
So we can only have both extensibility and autocompletion using this pattern.

Let's talk about this on Monday!


export interface WidgetsConfigByVocabulary {
'plone.app.vocabularies.Catalog': React.ComponentType;
}

export interface WidgetsConfigByFactory {
'Relation List': React.ComponentType;
'Relation Choice': React.ComponentType;
}

export interface WidgetsConfigByType {
boolean: React.ComponentType;
array: React.ComponentType;
object: React.ComponentType;
datetime: React.ComponentType;
date: React.ComponentType;
password: React.ComponentType;
number: React.ComponentType;
integer: React.ComponentType;
id: React.ComponentType;
}

export interface WidgetsConfigViewById {
file: React.ComponentType;
image: React.ComponentType;
relatedItems: React.ComponentType;
subjects: React.ComponentType;
}

export interface WidgetsConfigViewByWidget {
array: React.ComponentType;
boolean: React.ComponentType;
choices: React.ComponentType;
date: React.ComponentType;
datetime: React.ComponentType;
description: React.ComponentType;
email: React.ComponentType;
file: React.ComponentType;
image: React.ComponentType;
password: React.ComponentType;
relation: React.ComponentType;
richtext: React.ComponentType;
string: React.ComponentType;
tags: React.ComponentType;
textarea: React.ComponentType;
title: React.ComponentType;
url: React.ComponentType;
internal_url: React.ComponentType;
object: React.ComponentType;
}

export interface WidgetsConfigViewByType {
array: React.ComponentType;
boolean: React.ComponentType;
}

export interface WidgetsConfigViews {
getWidget: React.ComponentType;
default: React.ComponentType;
id: WidgetsConfigViewById;
widget: WidgetsConfigViewByWidget;
vocabulary: {};
choices: React.ComponentType;
type: WidgetsConfigViewByType;
}

export interface WidgetsConfig {
default: React.ComponentType;
id: {
schema: React.ComponentType;
subjects: React.ComponentType;
query: React.ComponentType;
recurrence: React.ComponentType;
remoteUrl: React.ComponentType;
id: React.ComponentType;
site_logo: React.ComponentType;
};
widget: {
textarea: React.ComponentType;
datetime: React.ComponentType;
date: React.ComponentType;
password: React.ComponentType;
file: React.ComponentType;
align: React.ComponentType;
buttons: React.ComponentType;
url: React.ComponentType;
internal_url: React.ComponentType;
email: React.ComponentType;
array: React.ComponentType;
token: React.ComponentType;
query: React.ComponentType;
query_sort_on: React.ComponentType;
querystring: React.ComponentType;
object_browser: React.ComponentType;
object: React.ComponentType;
object_list: React.ComponentType;
vocabularyterms: React.ComponentType;
image_size: React.ComponentType;
select_querystring_field: React.ComponentType;
autocomplete: React.ComponentType;
color_picker: React.ComponentType;
select: React.ComponentType;
};
vocabulary: {
'plone.app.vocabularies.Catalog': React.ComponentType;
};
factory: {
'Relation List': React.ComponentType;
'Relation Choice': React.ComponentType;
};
id: WidgetsConfigById;
widget: WidgetsConfigByWidget;
vocabulary: WidgetsConfigByVocabulary;
factory: WidgetsConfigByFactory;
choices: React.ComponentType;
type: {
boolean: React.ComponentType;
array: React.ComponentType;
object: React.ComponentType;
datetime: React.ComponentType;
date: React.ComponentType;
password: React.ComponentType;
number: React.ComponentType;
integer: React.ComponentType;
id: React.ComponentType;
};
views: {
getWidget: React.ComponentType;
default: React.ComponentType;
id: {
file: React.ComponentType;
image: React.ComponentType;
relatedItems: React.ComponentType;
subjects: React.ComponentType;
};
widget: {
array: React.ComponentType;
boolean: React.ComponentType;
choices: React.ComponentType;
date: React.ComponentType;
datetime: React.ComponentType;
description: React.ComponentType;
email: React.ComponentType;
file: React.ComponentType;
image: React.ComponentType;
password: React.ComponentType;
relation: React.ComponentType;
relations: React.ComponentType;
richtext: React.ComponentType;
string: React.ComponentType;
tags: React.ComponentType;
textarea: React.ComponentType;
title: React.ComponentType;
url: React.ComponentType;
internal_url: React.ComponentType;
object: React.ComponentType;
};
vocabulary: {};
choices: React.ComponentType;
type: {
array: React.ComponentType;
boolean: React.ComponentType;
};
};
type: WidgetsConfigByType;
views: WidgetsConfigViews;
}
1 change: 1 addition & 0 deletions packages/volto/news/5948.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve `ColorPickerWidget` typings @sneridagh
14 changes: 7 additions & 7 deletions packages/volto/src/components/manage/Widgets/ColorPickerWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { Form } from 'semantic-ui-react';
import { Grid, Button } from 'semantic-ui-react';
import { isEqual } from 'lodash';
import { isEmpty, isEqual } from 'lodash';
import { defineMessages, useIntl } from 'react-intl';

const messages = defineMessages({
Expand All @@ -26,11 +26,11 @@ type Color =
export type ColorPickerWidgetProps = {
id: string;
title: string;
value: string;
default: string;
required: boolean;
missing_value: unknown;
className: string;
value?: string;
default?: string | object;
required?: boolean;
missing_value?: unknown;
className?: string;
onChange: (id: string, value: any) => void;
colors: Color[];
};
Expand All @@ -41,7 +41,7 @@ const ColorPickerWidget = (props: ColorPickerWidgetProps) => {
const intl = useIntl();

React.useEffect(() => {
if (!props.value && props.default) {
if (isEmpty(props.value) && props.default) {
props.onChange(props.id, props.default);
}
// Yes, this is correct.
Expand Down
Loading