onizet / html2openxml

Html2OpenXml is a small .Net library that convert simple or advanced HTML to plain OpenXml components. This program has started in 2009, initially to convert user's comments from SharePoint to Word.
MIT License
306 stars 107 forks source link

Background color of text: highlighting or shading? #79

Open DynaSpan opened 3 years ago

DynaSpan commented 3 years ago

Currently, the implementation of background color on text is by using shading:

                        if (!colorValue.IsEmpty)
            {
                // change the way the background-color renders. It now uses Shading instead of Highlight.
                // Changes brought by Wude on http://html2openxml.codeplex.com/discussions/277570
                styleAttributes.Add(new Shading { Val = ShadingPatternValues.Clear, Fill = colorValue.ToHexString() });
            }

This makes sense if you're talking about table cell backgrounds etc. However, if your goal is to highlight text, users can become confused because in normal Word operations text is highlighted and not shaded.

My proposal is to implement a feature that checks if the color is applied on a span (meaning text; highlight) or on a different item, which would then shade it. I would like to know your opinions. If you agree, I will create a PR.

DynaSpan commented 3 years ago

I've created an implementation of this idea in my fork: feature/text-highlighting@ebce9dd

onizet commented 3 years ago

This is a good compromise between the original implementation and the change brought by Wude. I'm unsure whether it is a good idea to be too kind with users and try to find the nearest matching color. You will always find people complaining their color do not match. Also, your limited the behavior to only <span> and this could be a little less restricted (b,i,u,ins,del,) etc.. Maybe try the opposite condition and limit for p,table,pre,div.

DynaSpan commented 3 years ago

I'm unsure whether it is a good idea to be too kind with users and try to find the nearest matching color. You will always find people complaining their color do not match.

What would you suggest? Just casting the color name to the enum? And if it's a hex/rgb/hsl color just use the shading option?

Also, your limited the behavior to only and this could be a little less restricted (b,i,u,ins,del,) etc.. Maybe try the opposite condition and limit for p,table,pre,div.

Good idea. I should've time in the upcoming days to change this.

onizet commented 3 years ago

I like your idea to capitalize on the parsing of HtmlColor.

You could create an extension method on HtmlColor to convert to HighlightColorValues. Implement IEquatable<T> on HtmlColor to allow finding them in the Dictionary<HtmlColor, HighlightColorValues> mapping. (Just add the Interface because the method Equals already exists) You could also rely on HtmlColorTranslator.FromHtml(string namedColor) for easy setup.

If this is correctly described in the Wiki, I see no problem. I will handle that part myself.

I will just requires you to create a test case to validate your feature. This help me greatly to move forward to v3 and validate my refactoring. Thank you for your support

onizet commented 3 years ago

Hi DynaSpan, did you find some times to implement this ? I'm very interested in your proposal... If you don't know how to write the test case, I will handle it myself

DynaSpan commented 3 years ago

Hi Onizet,

I did some work a time ago, but since then I bought a house and it's been incredibly busy at work. I hope to find some time the upcoming weeks to look at the PR.

onizet commented 3 years ago

Thank you for your swift reply. Your situation sounds very familiar to me :)

jeevasusej commented 1 year ago

It's been one year. Do we have any update on this?