search

Stephanevg / PSHTML

Cross platform Powershell module to generate HTML markup language
https://pshtml.readthedocs.io/en/latest/
Other
167 stars 43 forks source link

Class [Color] is not available when using import-module #226

Closed Stephanevg closed 5 years ago

Stephanevg commented 5 years ago

It is impossible to use the static method [Class]::Red (green etc..) when the module is loaded using import-module.

On a higher note, it would make sense to perhaps have a function, that would return the color type

Get-PSHTMLColor -Type RGB -Color Red Returns -> rgb(220,20,60)

Add more types

wightsci commented 5 years ago

Hi Stephane,

I've been looking at this bug/feature request.

It appears that classes can't be accessed from a module without using 'Using module ', unless you wrap them in a function.

I've done a fairly major overhaul to the Color class, setting up a generic way of dealing with colors and adding the ability to output them in hex "#FFFFFF" format. The class has got support for the 140+ W3 color names (instead of those originally part of the module) but it does add 290+ lines to the module.

I've also created a Get-PSTMLColor function that generates rgb, rgba or hex format values based on a color name.

Regards,

Stuart.

Stephanevg commented 5 years ago

Hi @wightsci

Well that sounds like a pretty cool PR actually! Any chance you submit your changes back to the module? that would be really awesome! 👍

Stephanevg commented 5 years ago

Hi @wightsci Did you manage to do something on this one? I am close to ship a new version, and adding Get-PSHTMLColor cmdlet would be a very nice addition.

wightsci commented 5 years ago

I've just added PR #241. Let me know what you think.

Stephanevg commented 5 years ago

Really cool PR @wightsci ! Thank you so much for participating in this project!!!

I do have two small remarks (Actually a question and a remark) before I merge your PR:

1) I noticed you pushed to the wrong branch (you pushed directly to the master branch), could you please change it to the 'dev' branch please? that be great!

2) Also, I don't think I saw any pester tests for your code. are you planning to add some?

wightsci commented 5 years ago

I've rebased the PR to the dev branch as requested. Unfortunately I haven't created Pester tests. It would probably take another day or so. Would that be OK?

Stephanevg commented 5 years ago

Thanks @wightsci ! Yeah, that would be ok. (You don't need to write a test for every color. One test that generates a color for each of the values from the [ValidateSet("hex","hsl","hsla","rgb","rgba")] should be enough

Stephanevg commented 5 years ago

Also, I see the $type parameter is mandatory. Do you think it would make sense to set it as mandator= $false, and propose a default value ( RGB perhaps, as it is the most common one?)

It would make it easier for the users that don't know what they actually want. don't you think?

Stephanevg commented 5 years ago

One last thing, could you also add a short comment based help section to your function?

it helps to auto generate the end-users documentation. You can find an example here --> https://github.com/Stephanevg/PSHTML/blob/master/Code/Functions/Public/New-PSHTMLChartBarDataSet.ps1

wightsci commented 5 years ago

Hi,

There's already comment-based help for Get-PSHTMLColor (lines 1696 to 1739). I'm happy to change $type to default to rgb.

wightsci commented 5 years ago

Apologies, put the comment-based help in the wrong part of the function.

wightsci commented 5 years ago

Added new commit to PR with Pester tests and a fixed hsl calculation that was identified by the Pester tests!

Stephanevg commented 5 years ago

This is really great work that you did here @wightsci I just merged the PR https://github.com/Stephanevg/PSHTML/pull/249/ which contained the ArgumentCompleter for the Get-PSHTMLColor function. So, I think this issue is complete now. Thank you again for you great work!