Skip to content

Conversation

@dmeranda
Copy link

This is my attempt to overhaul the color handling internals in Prawn. It lets you use nearly all the CSS syntax to represent colors. It also introduces a Prawn::PDFColor base class to store instances of specific colors; of which a subclass Prawn::CSSColor contains all the CSS logic.

I also updated the manual to include a new section on colors in the Basic Concepts chapter.

I think this is well documented and tested, though since its not a trivial change more eyes would be appreciated.

@packetmonkey
Copy link
Contributor

So I dropped everything and looked over this PR, loving the idea.

I personally would like to see the various classes defined in their own files to make things easier it find.

It looks like to me you are using PDFColor as an abstract class ( as commented ), I wonder if instead of returning values that aren't ment to really be used in the system, those methods should raise NotImplementedError instead. That's a higher level style decision @sandal should sign off on I imagine.

Also not sure about presenting CSS color as the 'standard' as inferred in manual/basic_concepts/css_colors.rb but for a new user that may make the learning curve easier, and it's not like we dropped support for the other forms.

👍 from me having not yet pulled the branch down and played with it myself.

@packetmonkey
Copy link
Contributor

Also are there specs in this PR for the new code or am I just not seeing them?

@practicingruby
Copy link
Member

@packetmonkey, @dmeranda: Right now I'm looking to reduce the amount of features we're directly supporting in Prawn itself, so I'd much rather see this supported as a third-party extension rather than consider a pull request.

Happy to consider a discussion around what extension points we'd need to add to the API to facilitate that!

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants