GrapesJS / mjml

Newsletter Builder with MJML components in GrapesJS
http://grapesjs.com/demo-mjml.html
BSD 3-Clause "New" or "Revised" License
618 stars 218 forks source link

Added colorScheme option and customClasses options for blocks #322

Closed gautamgiri-dev closed 1 year ago

gautamgiri-dev commented 1 year ago
DRoet commented 1 year ago

Hi,

Thanks for reaching out. Unfortunately I can't merge these changes as is.

1) you should probably use custom blocks to add your own custom classes when you need more customization. I could agree to an option, but the current design needs a lot of work since it is unclear that vector === media and class === attributes.class. Again, probably easier to just use custom blocks on your end.

2) I can't merge the theme changes since it is a breaking change. I'm not a fan of removing the useCustomTheme option and replacing it with something else. Perhaps we can extend it by also allowing an object to be passed down such as:

useCustomTheme: {
    primaryColor?: string,
    secondaryColor?: string,
    quaternaryColor?: string
},
gautamgiri-dev commented 1 year ago

Sure @DRoet, I understand your concerns and we can surely extend the useCustomTheme option.

Also, regarding the blocks I want to provide options to easily add custom classes to the default blocks. I will be refactoring the prop names with something more sensible. Since it's easier for users to customize the existing blocks with look and feel of their choice rather than creating every block as a custom block. Apart from that, I am also thinking to add options to easily customize the svg icons of the default blocks which can be a great timesaver for the developers.

artf commented 1 year ago

Yeah this can't be merged due to issues already mentioned by @DRoet