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

List bullets associated with colored text are not colored #409

Open
jeffreyyoung opened this issue Jul 8, 2015 · 17 comments
Open

List bullets associated with colored text are not colored #409

jeffreyyoung opened this issue Jul 8, 2015 · 17 comments

Comments

@jeffreyyoung
Copy link

The bullet should be colored red to match the text
image

This issues occurs with both number and bullet lists

@joaovpmamede
Copy link

Having the same problem.
Also when changing font size the bullet does not change its size (would this be possible?).

Is this something that we can expect to be introduced on this quill version or only in parchment?

@jhchen
Copy link
Member

jhchen commented Sep 2, 2015

This would likely be post-parchment unless someone from the community decides to take it on.

@Natim
Copy link

Natim commented Jun 16, 2017

I looked a bit into that, it might be the way that list are handled in Quill.js

{
    "ops": [
        {
            "attributes": {
                "size": "huge"
            },
            "insert": "18px"
        },
        {
            "attributes": {
                "list": "ordered"
            },
            "insert": "\n"
        },
        {
            "attributes": {
                "size": "large"
            },
            "insert": "16px"
        },
        {
            "attributes": {
                "list": "ordered"
            },
            "insert": "\n"
        }
    ]
}

As you can see the ordered list doesn't contain information about the size (or the color) which makes the rendered HTML not styled.

Any idea why it is not rendered as:

{
    "ops": [
        {
            "attributes": {
                "list": "ordered",
                "size": "huge"
            },
            "insert": "18px\n"
        },
        {
            "attributes": {
                "list": "ordered",
                "size": "large"
            },
            "insert": "16px\n"
        }
    ]
}

@Natim
Copy link

Natim commented Jun 16, 2017

@jhchen How would you like the community to fix that? By adding information about style to the node containing the list attribute? By merging both nodes (it will probably break a lot of things)?

@slab slab deleted a comment from vingrad Jun 19, 2017
@slab slab deleted a comment from MaximusBaton Jun 19, 2017
@slab slab deleted a comment from zgayjjf Jun 19, 2017
@jhchen
Copy link
Member

jhchen commented Jun 19, 2017

@Natim You are right one main issue is color (and size) are inline level styles and not block level. You can add a block level color style quite trivially but a naive implementation would result in very confusing usability without thinking through some edge cases.

  1. What happens of half the text of a list item is red and half is blue? Should the bullet inherit the color of the first half?
  2. Colors are implemented with classes and styles. Will a block level implementation also support both?
  3. How should the HTML look (affected by the answer to 2.)? If we do support an inline style implementation, how do we deal with the fact that the descendant text nodes will inherit this color? What if I just want the bullet to be red but some text to be the default black color?

The next step would be to think through how it will look in the JSON Delta representation and how that maps to HTML, given an inline style only implementation and a class only implementation (or decide that one would not be supported), and propose that solution.

@Natim
Copy link

Natim commented Jun 19, 2017

What happens of half the text of a list item is red and half is blue? Should the bullet inherit the color of the first half?

Yes

Colors are implemented with classes and styles. Will a block level implementation also support both?

Yes why not?

If we do support an inline style implementation, how do we deal with the fact that the descendant text nodes will inherit this color?

They will inherit the color and you will be able to change it for descendant nodes If you wish (by selecting them and changing their color again)

What if I just want the bullet to be red but some text to be the default black color?

This is out of scope I guess because it is a new feature while we are trying to address a bug.
But I guess you could start by putting some text in red and then select the text to turn it in black?

I wouldn't be to worried about edge cases as far as one can get out of them by removing the wrong nodes and trying again.

@jhchen
Copy link
Member

jhchen commented Jun 26, 2017

Thinking about edge cases are critical to designing a solution. Not all parts have to be implemented at the same time but ignoring them altogether brings the risk that you will have to redesign and reimplement everything.

@Natim
Copy link

Natim commented Jun 26, 2017

What is the current behavior for thoose use cases? Do you want to cover them any time soon? I don't think we should plan ahead for things we don't want to support.

@jhchen
Copy link
Member

jhchen commented Jun 26, 2017

The current behavior is the bullet is always black which has been acknowledged as a bug. So I'm clarifying in what cases should it not be black and in those cases how does the data and markup look like. This seems like an obvious prerequisite to an implementation to me.

There are higher priority core issues so to move this forward someone that needs this enough should think through the cases and make a clear proposal if they need someone else to implement it. Otherwise they can implement a solution themselves but without discussing it would run the risk of not having the PR accepted.

@aikar
Copy link

aikar commented Jun 28, 2017

This should behave like every other word processor, the way Word implements it is that:
If every bit of text is selected for that list item, then styles apply surrounding the LI itself.
If only part of the text is selected, only the selected text (and not the bullet) is changed.

But when all of it is, the style choices (size, color, etc), should also apply to the LI, but not the list itself.

You may want to have some bullets blue and some red, so it has to wrap/apply to the LI.

@jhchen
Copy link
Member

jhchen commented Jul 16, 2017

Thank for you the proposed behavior @aikar. From an implementation perspective it is very helpful because this suggests no changes need to be made at the data layer with no additional information needs to be stored to determine list coloring (or size, font, etc) which reduces the development cost considerably.

I would suggest as a starting point for those interested in helping to take a look at ListItem in formats/list.js and adding an optimize() function to it that checks if all children has the same color and if so add that to the list item itself and if not remove. The same strategy can be applied to font, size, and background.

@ghost
Copy link

ghost commented Jan 24, 2018

I had to implement this, so here you go - some basic working implementation:

IMPORTANT: This assumes you use built in color format (you can always modify the code to work with any other) and that you want bullet points to be colored the same color as text only if whole list entry content is the same color!

ALSO IMPORTANT: you have to add bullet points unicode character using li::before and "content" attribute in CSS.

  1. Register your custom font color attributor same as built-in but BLOCK sized, because ListItem blot will block INLINE styles and you will search for hours what is going on:
class CustomColor extends Parchment.Attributor.Style {
  value(node) {
    let value = super.value(node);

    if (!value.startsWith('rgb(')) return value;

    value = value.replace(/^[^\d]+/, '').replace(/[^\d]+$/, '');

    return '#' + value.split(',').map(function(component) {
      return ('00' + parseInt(component, 10).toString(16)).slice(-2);
    }).join('');
  }
}

const customColorAttributor = new CustomColor('custom-color', 'color', {
    scope: Parchment.Scope.BLOCK,
});

Quill.register(customColorAttributor);
  1. Extend "ListItem" blot (its the built-in blot for li elements) and overload optimize on it with following code:
optimize(context) {
    super.optimize(context);

    if (this.children.length === 1) {
      const child = this.children.head;
      const attributes = child.attributes;

      if (attributes && attributes.attributes.color) {
        const color = attributes.attributes.color.value(child.domNode);
        super.format('custom-color', color);
      }
    } else {
      if (this.attributes.attributes.hasOwnProperty('custom-color')) {
        super.format('custom-color', null);
      }
    }
}

I don't know to what degree this is valid with Quill/Parchment philosophy but documentation is pretty scarce so this is the way I solved it for my case.

@natterstefan
Copy link

Hi @jakub-pietras-codete,

I need to say "sorry" upfront because my question is not exactly related to this issue, but you might be able to help me out with this and turn a blind eye to this comment.

How can I extend a ListItem blot and add a custom class to each li element?

Context: I need a proper css selector for li elements on the first level, by ignoring other li elements with eg. a ql-indent-<n> class. Thanks.

@ghost
Copy link

ghost commented Dec 21, 2018

Extending ListItemBlot:
const ListItemBlot = Quill.import('formats/list/item');
then
class CustomListItem extends ListItemBlot {}

For your case, I don't exactly understand what you want to achieve. If you have nested lists then in one of the li's you need to have another ul/ol tag and then more children deeper. You can just manually use direct descendant .something > li selector to access the first level. So if the output is in container with .x class then just do .x > li. If that is not enough, which sounds like might be in your case - extend the ListItem as above and overload built-in methods to add the class name in certain situations. I think that could go into the optimize metod, I don't really remember, last worked with this a year ago.

@natterstefan
Copy link

Hi @jakub-pietras-codete, thanks for your help. I'll dig into this and try it. The reason why I ask is because of quill's approach of how to format list and nested lists: Every list item is in the same ol, only nested list items have a special class attached: ql-indent-1.

That's when I ran into troubles selecting only the first level (without any ql-indent-x class.

@haykkh
Copy link

haykkh commented Mar 29, 2023

I had to implement this, so here you go - some basic working implementation:

IMPORTANT: This assumes you use built in color format (you can always modify the code to work with any other) and that you want bullet points to be colored the same color as text only if whole list entry content is the same color!

ALSO IMPORTANT: you have to add bullet points unicode character using li::before and "content" attribute in CSS.

  1. Register your custom font color attributor same as built-in but BLOCK sized, because ListItem blot will block INLINE styles and you will search for hours what is going on:
class CustomColor extends Parchment.Attributor.Style {
  value(node) {
    let value = super.value(node);

    if (!value.startsWith('rgb(')) return value;

    value = value.replace(/^[^\d]+/, '').replace(/[^\d]+$/, '');

    return '#' + value.split(',').map(function(component) {
      return ('00' + parseInt(component, 10).toString(16)).slice(-2);
    }).join('');
  }
}

const customColorAttributor = new CustomColor('custom-color', 'color', {
    scope: Parchment.Scope.BLOCK,
});

Quill.register(customColorAttributor);
  1. Extend "ListItem" blot (its the built-in blot for li elements) and overload optimize on it with following code:
optimize(context) {
    super.optimize(context);

    if (this.children.length === 1) {
      const child = this.children.head;
      const attributes = child.attributes;

      if (attributes && attributes.attributes.color) {
        const color = attributes.attributes.color.value(child.domNode);
        super.format('custom-color', color);
      }
    } else {
      if (this.attributes.attributes.hasOwnProperty('custom-color')) {
        super.format('custom-color', null);
      }
    }
}

I don't know to what degree this is valid with Quill/Parchment philosophy but documentation is pretty scarce so this is the way I solved it for my case.

To build on this solution up top, we implemented a fix for font family/size/color like so:

const Parchment = Quill.import('parchment')

const customFontFamilyAttributor = new Parchment.Attributor.Style('custom-family-attributor', 'font-family')
const customSizeAttributor = new Parchment.Attributor.Style('custom-size-attributor', 'font-size')
const customColorAttributor = new Parchment.Attributor.Style('custom-color-attributor', 'color')

const ListItemBlot = Quill.import('formats/list/item')

class CustomListItem extends ListItemBlot {
  optimize(context) {
    super.optimize(context)

    if (this.children.length >= 1) {
      const child = this.children.head
      const attributes = child?.attributes?.attributes

      if (attributes) {
        for (const key in attributes) {
          const element = attributes[key]
          let name = element.keyName
          const value = element.value(child.domNode)

          if (name === 'color') super.format('custom-color-attributor', value)
          else if (name === 'font-family') super.format('custom-family-attributor', value)
          else if (name === 'font-size') super.format('custom-size-attributor', value)
        }
      }
    }
  }
}

Quill.register(customColorAttributor, true)
Quill.register(customFontFamilyAttributor, true)
Quill.register(customSizeAttributor, true)
Quill.register(CustomListItem, true)

@gdutwyg
Copy link

gdutwyg commented Jul 27, 2023

To build on this solution up top, we implemented a fix for font family/size/color like so:

const Parchment = Quill.import('parchment')

const customFontFamilyAttributor = new Parchment.Attributor.Style('custom-family-attributor', 'font-family')
const customSizeAttributor = new Parchment.Attributor.Style('custom-size-attributor', 'font-size')
const customColorAttributor = new Parchment.Attributor.Style('custom-color-attributor', 'color')

const ListItemBlot = Quill.import('formats/list/item')

class CustomListItem extends ListItemBlot {
  optimize(context) {
    super.optimize(context)

    if (this.children.length >= 1) {
      const child = this.children.head
      const attributes = child?.attributes?.attributes

      if (attributes) {
        for (const key in attributes) {
          const element = attributes[key]
          let name = element.keyName
          const value = element.value(child.domNode)

          if (name === 'color') super.format('custom-color-attributor', value)
          else if (name === 'font-family') super.format('custom-family-attributor', value)
          else if (name === 'font-size') super.format('custom-size-attributor', value)
        }
      }
    }
  }
}

Quill.register(customColorAttributor, true)
Quill.register(customFontFamilyAttributor, true)
Quill.register(customSizeAttributor, true)
Quill.register(CustomListItem, true)

thanks your solution. I think the code can add else condition。@haykkh

    if (attributes) {
        for (const key in attributes) {
          const element = attributes[key]
          let name = element.keyName
          const value = element.value(child.domNode)

          if (name === 'color') super.format('custom-color-attributor', value)
          else if (name === 'font-family') super.format('custom-family-attributor', value)
          else if (name === 'font-size') super.format('custom-size-attributor', value)
        }
     } else {
        super.format('custom-color-attributor', false)
        super.format('custom-family-attributor', false)
        super.format('custom-size-attributor', false)
      }

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

No branches or pull requests

8 participants