Skip to content

InNumbers component interface #112

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

InNumbers component interface #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,22 @@ interface CustomCodeComponent extends Node {
- The basic interface in Spark to make reference to this system above (eg. the git repo URL or a public S3 bucket), and provide some data for it if necessary. This will be the Custom Component storyblock.
- The data Spark receives from entering a specific ID will be used to render dynamic fields (the `attributes`).

### InNumbers

```ts
interface InNumbersNumber extends Node {
type: "in-numbers-number"
numberLabel: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
numberLabel: string
number: string

Should we keep the field name consistent with BigNumber?
(Incidentally I was wondering if it should be a number rather than string, but looks like BigNumber is also a string!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea of using numberLabel is because I was leaving number available in case one day we will have a proper number: number, similarly to here we may have a date: Date.

If we want to communicate consistency/relations perhaps we should do this:

interface Number extends Node {
        number: string; << Still this is confusing to me
        description: string;
}

interface BigNumber extends Number {
       type: "big-number";
}

interface InNumbers extends Parent {
	type: "in-numbers"
	children: [Heading, Number, Number?, Number?]
}

but I wonder if this become a slippery slope towards some kind of coupling hell

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adgad On second thought, the likelihood to need a number as number seems very low, so we can go with your suggestion and same perhaps for the date in the Timeline PR. At then end, as UI representation, the need of have better types it's small. In the long term though we may have issue to automatically associate for instance a datePicker in Spark to a field that is a string instead of a proper date, that kind of problems.

Copy link
Collaborator

@adgad adgad Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

👍 I guess because we haven't had any validation till now in the CMS, it's very likely that we have existing timeline dates that aren't dates, big numbers that aren't numbers etc.

In the long term though we may have issue to automatically associate for instance a datePicker in Spark to a field that is a string instead of a proper date, that kind of problems.

yeah i can see this being useful with schema-generated components in the future, and being able to have validation in the CMS as well!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's very likely that we have existing timeline dates that aren't dates, big numbers that aren't numbers

Yep. The options are:

  1. Use the label number as a string in the new In Numbers component and use date as a string in the new Timeline component
  • Pros: consistency with existing components
  • Cons: It's confusing and it will prevent us one day to have proper number as number and date as Date and we may end-up in even more confusing choices like numberAsNumber: number
  1. Use label for fields that are free string, Editorial can really put whatever they want there
  • Pros: avoid future breaking change or increase of what seems unideal/confusing naming convention
  • Cons: lost in consistency with existing nodes

IMO and this is also for @elpopova and @todor-videv1 perhaps, part of this topic is about which future do we see for the Content Tree as a document? Is it just for a UI with a bunch of strings or shall it evolve to something that machine should better understand too. I'm raising it because this is in some form related too

Copy link

Choose a reason for hiding this comment

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

would keeping it to a generic thing like value help for future modelling for simple types (as opposed to data structures)? e.g. Number.value, SimpleThing.value. Or a bit more specific like text for strings.

Copy link
Contributor Author

@umbobabo umbobabo Oct 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you @debugwand. I think this is a very good suggestion. It's already applied for Text here.

Potentially, we could use value and not having a breaking change even if we move towards what I mentioned here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually, we could have something like...

interface Number extends Node {
        value: number; 
        number: string; 
        description: string;
}

interface BigNumber extends Node {
      type: "big-number";
} && Pick<Number, 'number', 'description' >

interface InNumbersNumber extends Node {
      type: "in-numbers-number";
} && Pick<Number, 'value', 'description' >

interface InNumbers extends Parent {
	type: "in-numbers"
	children: [Heading, Number, Number?, Number?]
}

... but we may not get any particular value to tight to a single Number node at this stage, perhaps we can keep them separated as they are in this PR and use value that will still permit us in the future to merge the types without a breaking change if we desire to do so

description: string
}

interface InNumbers extends Parent {
type: "in-numbers"
children: [Heading, InNumbersNumber, InNumbersNumber?, InNumbersNumber?]
}
```

- **InNumbers** represents a set from 1 to 3 numbers with related descriptions, introduced by a Heading

## License

Expand Down
36 changes: 36 additions & 0 deletions content-tree.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,15 @@ export declare namespace ContentTree {
/** Configuration data to be passed to the component. */
attributes: CustomCodeComponentAttributes;
}
interface InNumbersNumber extends Node {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We agreed with @adgad to use BigNumber instead of this (FE may need to be reviewed because styles used by big number)

type: "in-numbers-number";
numberLabel: string;
description: string;
}
interface InNumbers extends Parent {
type: "in-numbers";
children: [Heading, InNumbersNumber, InNumbersNumber?, InNumbersNumber?];
Copy link
Contributor Author

@umbobabo umbobabo Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Heading it's not mandatory and we need to support ImageSet

Suggested change
children: [Heading, InNumbersNumber, InNumbersNumber?, InNumbersNumber?];
children: [Heading?, ImageSet?, InNumbersNumber, InNumbersNumber?, InNumbersNumber?];

}
namespace full {
type BodyBlock = Paragraph | Heading | ImageSet | Flourish | BigNumber | CustomCodeComponent | Layout | List | Blockquote | Pullquote | ScrollyBlock | ThematicBreak | Table | Recommended | Tweet | Video | YoutubeVideo | Text;
type LayoutWidth = "auto" | "in-line" | "inset-left" | "inset-right" | "full-bleed" | "full-grid" | "mid-grid" | "full-width";
Expand Down Expand Up @@ -558,6 +567,15 @@ export declare namespace ContentTree {
/** Configuration data to be passed to the component. */
attributes: CustomCodeComponentAttributes;
}
interface InNumbersNumber extends Node {
type: "in-numbers-number";
numberLabel: string;
description: string;
}
interface InNumbers extends Parent {
type: "in-numbers";
children: [Heading, InNumbersNumber, InNumbersNumber?, InNumbersNumber?];
}
}
namespace transit {
type BodyBlock = Paragraph | Heading | ImageSet | Flourish | BigNumber | CustomCodeComponent | Layout | List | Blockquote | Pullquote | ScrollyBlock | ThematicBreak | Table | Recommended | Tweet | Video | YoutubeVideo | Text;
Expand Down Expand Up @@ -824,6 +842,15 @@ export declare namespace ContentTree {
/** How the component should be presented in the article page according to the column layout system */
layoutWidth: LayoutWidth;
}
interface InNumbersNumber extends Node {
type: "in-numbers-number";
numberLabel: string;
description: string;
}
interface InNumbers extends Parent {
type: "in-numbers";
children: [Heading, InNumbersNumber, InNumbersNumber?, InNumbersNumber?];
}
}
namespace loose {
type BodyBlock = Paragraph | Heading | ImageSet | Flourish | BigNumber | CustomCodeComponent | Layout | List | Blockquote | Pullquote | ScrollyBlock | ThematicBreak | Table | Recommended | Tweet | Video | YoutubeVideo | Text;
Expand Down Expand Up @@ -1105,5 +1132,14 @@ export declare namespace ContentTree {
/** Configuration data to be passed to the component. */
attributes?: CustomCodeComponentAttributes;
}
interface InNumbersNumber extends Node {
type: "in-numbers-number";
numberLabel: string;
description: string;
}
interface InNumbers extends Parent {
type: "in-numbers";
children: [Heading, InNumbersNumber, InNumbersNumber?, InNumbersNumber?];
}
}
}
Loading