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

Background color of text: highlighting or shading? #79

Open
DynaSpan opened this issue Sep 21, 2020 · 8 comments
Open

Background color of text: highlighting or shading? #79

DynaSpan opened this issue Sep 21, 2020 · 8 comments

Comments

@DynaSpan
Copy link
Contributor

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
Copy link
Contributor Author

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

@onizet
Copy link
Owner

onizet commented Sep 23, 2020

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
Copy link
Contributor Author

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
Copy link
Owner

onizet commented Sep 24, 2020

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
Copy link
Owner

onizet commented Jan 21, 2021

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
Copy link
Contributor Author

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
Copy link
Owner

onizet commented Jan 22, 2021

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

@jeevasusej
Copy link

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

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