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.

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

Migrate to single layer text styling #3

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Migrate to single layer text styling #3

wants to merge 24 commits into from

Conversation

timhaywood
Copy link
Member

Pretty much a full rewrite by @fartinmartin to make use of the new CC 2025 text style expressions, where you can set the style for a range of characters.

This enables markdown type rendering on a single text layer.

@timhaywood
Copy link
Member Author

timhaywood commented Oct 21, 2024

I only updated the deps really and moved stuff into the main files. expression-globals-typescript should have all the correct expression types now.

@fartinmartin had a few questions, let me know what you think. Love the use of the reducer btw!

  1. Was there a specific reason you return a function to execute rather than the final style object directly? Not sure I see any benefits personally.
  2. How do you feel about having markdown as the first param to parse(), with the options after? Makes it a little easier to

I feel like a simpler API of parse(markdown, options): TextStyle is a little more intuitive.

@timhaywood timhaywood marked this pull request as draft October 21, 2024 05:46
@fartinmartin
Copy link
Collaborator

Woo! I can't remember exactly my reasoning for returning a render function, it may have been a hesitancy to auto run eval for the user? Totally aligned to remove that as well as to update the API to parse(markdown, options): TextStyle!

@timhaywood timhaywood marked this pull request as ready for review October 22, 2024 03:19
@timhaywood
Copy link
Member Author

@fartinmartin updated a few things, let me know what you think.

I rewrote the API to be simply parse(markdown: string, parsers: Parser[]): TextStyle. This meant:

  • No more baseStyles, the base style is now inherited from the existing text.sourceText.style
  • No more fontMap, it felt a lot more simple/intuitive to treat fonts the same as the rest of the styles. So you just add them in as styles on a custom parser if you want to change the font
  • Return the TextStyle directly rather than a function to get it
  • Remove object passed paramaters, since there's only two

Quite a change, but think it all makes sense 😅 You might want to test it out, make sure I didn't break anything.

Updated the README, but still need to do the tests.

@fartinmartin
Copy link
Collaborator

fartinmartin commented Oct 22, 2024

Oh nice! I like the idea of simplifying the API. However, maybe there's a way to keep some of the baseStyle/fontMap functionality? For example, if I want to use Menlo in place of Verdana I would need to:

  1. Set the "regular" font by selecting "Menlo Regular" via the UI.
  2. Set the "bold", "italic", and heading fonts via custom parser overrides for each of the default parsers.

So, my expression is now (unless I'm missing something!):

const { parse } = footage("style-parser.jsx").sourceData.get();

const textLayer = thisComp.layer("Text:Input");
const inputText = textLayer.text.sourceText.value;

// can't set "regular" style via expressions—must set via UI
// thisLayer.text.sourceText.style.setFont("Menlo-Regular");

const defaultOverrides = [
  {
    name: "bold",
    styles: { font: "Menlo-Bold" },
  },
  {
    name: "italic",
    styles: { font: "Menlo-Italic" },
  },
  {
    name: "h1",
    styles: { font: "Menlo-Bold" },
  },
  {
    name: "h2",
    styles: { font: "Menlo-Bold" },
  },
  {
    name: "h3",
    styles: { font: "Menlo-Bold" },
  },
  {
    name: "h4",
    styles: { font: "Menlo-Bold" },
  },
  {
    name: "h5",
    styles: { font: "Menlo-Bold" },
  },
  {
    name: "h6",
    styles: { font: "Menlo-Bold" },
  },
];

const customParsers = [
  ...defaultOverrides,
  {
    style: "highlight",
    matcher: /==(.+?)==/g,
    styles: { fillColor: [1, 1, 0] },
  },
];

parse(inputText, customParsers);

Which feels a little heavy on boilerplate, maybe? What do you think?

ae-md.zip

Also, I do like the ability to set base styles via expressions as it's more explicit than relying on the UI settings, but maybe more importantly—it'd be nice to have dynamic control of base styles.

@timhaywood
Copy link
Member Author

Yeah good point! Totally see what you mean. Adding in control over the default fonts makes sense, so you don't have to do each one manually. I'll add fontMap back in.

As for the base style agree setting the styles with expressions is often better then with the UI so we should have a way to allow that.

I think my intuition would be to handle this outside of the parse function? I feel like parse should be scoped to the markdown specific work, but on the other hand your StyleDefinition API is nicer than calling lots of .set methods..

How do you feel about optionally parsing in a TextStyle object? That way you could create one with createStyle(), inherit it from a different layer, or modify the existing one.

const { parse } = footage("style-parser.jsx").sourceData.get();
const textStyle = thisLayer.text.sourceText.style.setFont("Arial");
parse(value, { textStyle });

@timhaywood
Copy link
Member Author

timhaywood commented Oct 22, 2024

So the API would be:

parse(markdown: string,
  parsers?: Parsers[],
  fontMap?: FontMap,
  textStyle?: TextStyle,
}): TextStyle;

@timhaywood
Copy link
Member Author

@fartinmartin okay updated the API, let me know what you think. Pretty sure it's all working well!

@fartinmartin
Copy link
Collaborator

@timhaywood hey, sorry for the delay! This looks great! Coming back to this code I felt like I needed to clarify some of the type names to help make sense of things—hope that's ok.

I think the API you've landed on makes a lot of sense. Re: the StyleDefinition API, what if we exported an additional function setStyles (or similar, not set on that name) as a little wrapper for users to use when creating their textStyle? For example:

const { parse, setStyles } = footage('style-parser.jsx').sourceData.get();

const inputText = thisComp.layer("Input").text.sourceText.value;

const textStyle = setStyles({
  font: "ArialMT",
  fontSize: 20,
});

parse(inputText, { textStyle });

Here's an example project with that idea: style-parser.zip

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.

2 participants