Skip to content

Conversation

@umbobabo
Copy link
Contributor

@umbobabo umbobabo commented Oct 8, 2025

Representation for an In numbers component

image

This is related to the issue about moving away from the Layout component

README.md Outdated
```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

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

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

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

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

@umbobabo umbobabo Oct 14, 2025

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?];

/** 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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As mentioned here we don't want to spend too much time untangling the BigNumber issue but we may want to do that in the future. I think we may go back to the initial idea for now of separate them. Following, is an evolved version of the initial idea. I have abstracted a bit the Definition because it doesn't really need to be specifically for a number only (At least by the code perspective, although we want to keep it specific by the Editorial POV):

/**
 * A definition has a term and a related description
 */
interface Definition extends Node {
	type: "definition"
	term: string
	description: string
}

/**
 * InNumbers represents a set of numbers with related descriptions
 */
interface InNumbers extends Parent {
	type: "in-numbers"
	/** The title for the InNumbers */
	title: string
	children: [Definition, Definition, Definition]
}

In the future, we may have a new Big Number that uses just the Definition or perhaps definition as a child. The problem of having nodes relating to the same child is about how the FE is going to handle it. Let say we want to achieve a semantic html like the following.

// A potential new Big Number
<aside>
  <dl>
      <dt>111</dt> 
      <dd>Venenatis urna massa</dd>
  </dl>
</aside>
// The new InNumbers
<section data-type="in-numbers">
  <h3>Aliquam venenatis urna massa</h3>
  <dl>
    <dt>111</dt>
    <dd>Venenatis urna massa</dd>
    <dt>222</dt>
    <dd>Aliquam urna</dd>
    <dt>666</dt>
    <dd>Unta us urna massa</dd>
  </dl>
</section>

If you want to achieve this reusing a shared Definition node that become a component, you still need a new Big Number, you cannot use a Definition by itself because of the way we are wrapping into the

tag.
Definition will be something like a fragment component...

      <dt>111</dt>
      <dd>Venenatis urna massa</dd>

while the bigNumber and the inNumbers will use it as a child...

// New Big Number (NumberDefition see below)
<aside>
  <dl>
      {children}
  </dl>
</aside>
// InNumbers
<section data-type="in-numbers">
  <h3>Aliquam venenatis urna massa</h3>
  <dl>
    {children}
   </dl>
</section>

The fragment is a bit awkward because it's not a fully self-contained component, frankly, I'm not sure this is a good idea. However, the new bigNumber will have similar interface than inNumbers:

/**
 * A potential new BigNumber
 */
interface NumberDefinition extends Parent {
	type: "number-definition"
	children: [Definition]
}

@adgad @debugwand @andy-little It seems safe to me to move on avoiding to share the sub-node with BigNumer and think at the rest on a later stage. What do you think?

Choose a reason for hiding this comment

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

Sounds good to me. Our schema in spark is sort of a contract between contentTree, bodyXML, prosemirror and react - so I'm not sure a refactor would make much sense on our end, but when you're ready to define the new output for bigNumber we can easily support that 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants