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

RichEditBox dont match Theme #3061

Closed
CesGan opened this issue Aug 6, 2020 · 10 comments
Closed

RichEditBox dont match Theme #3061

CesGan opened this issue Aug 6, 2020 · 10 comments
Labels
area-TextBox TextBox, RichEditBox

Comments

@CesGan
Copy link

CesGan commented Aug 6, 2020

Describe the bug
If you load a text to a RichEditBox when the Theme is in Dark mode the text will be black.

Steps to reproduce the bug

  1. Just use the App XAML Controls Gallery version 1.2.16.0 and go to the RichEditBox control.
  2. Go to the third example where you can upload a text file.
    image
  3. Make sure you are on dark mode and upload the text.

Expected behavior
The text should be white.

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Aug 6, 2020
@marcelwgn
Copy link
Contributor

I am not sure if this is actually a bug. Per default text is black, so on black background you don't see anything. Similarly, if you load a file with white text into a RichEditBox in light theme, you can't read the text either.

@StephenLPeters
Copy link
Contributor

Yeah, I think this is by design. With RichEdit, the text color is associated with the text itself so when you copy black text into a text box its supposed to be black. The background is also black though so you get this behavior. We could have xaml controls gallery be responsive to this scenario but I'm not sure that is worth it. @stmoy

@StephenLPeters StephenLPeters added area-TextBox TextBox, RichEditBox team-Framework needs-triage Issue needs to be triaged by the area owners and removed needs-triage Issue needs to be triaged by the area owners labels Aug 6, 2020
@stmoy
Copy link
Contributor

stmoy commented Aug 6, 2020

The bug reported here doesn't repro on other RichEditBoxes - just this specific one that attempts to emulate a text editor (and specifically sets the text color to black). I think it'd be reasonable to move this over to the XCG repo. @chingucoding did you add this sample? Maybe instead of setting the color to black, it could be set to a ThemeBrush instead?

-=-=-=-=-

image

@marcelwgn
Copy link
Contributor

There are two things here:

  1. When loading a RTF file, the RichEditBox not only copies the text, but also the font colors, resulting in a rtf file with black text not being readable on dark theme.
  2. There were a few changes made to the samples to deal with the lighttheme on focus, which are not up to date anymore. I am working on reverting those changes and will create a PR for this soon.

Regarding 1, I don't think there is much to do beside overriding the font color delivered to us through the rtf file, @stmoy do you think we should do that kind of changes when loading an rtf file?

@stmoy
Copy link
Contributor

stmoy commented Aug 7, 2020

For (1), I don't think we should do any sort of shenanigans to the copied text in the sample. I think the absolute easiest thing would be to change the background color back to white just for this sample.

@StephenLPeters
Copy link
Contributor

StephenLPeters commented Aug 7, 2020

For (1), I don't think we should do any sort of shenanigans to the copied text in the sample. I think the absolute easiest thing would be to change the background color back to white just for this sample.

I think you would still hit this issue when loading an rtf file with white text. Maybe that is a little less common though?

@stmoy
Copy link
Contributor

stmoy commented Aug 7, 2020

I think you would still hit this issue when loading an rtf file with white text. Maybe that is a little less common though?

True, and probably. This isn't mean to be a proper editor, just a simulacrum of one. It's ok if the edge cases don't work.

@marcelwgn
Copy link
Contributor

If we change the background, we also need to adjust the foreground, foregroundhover, foregroundfocused ... values. I am not sure if we want to go down that road, especially as it suggest that you should use the RichEditBox with changed background and foreground values.

@stmoy
Copy link
Contributor

stmoy commented Aug 7, 2020

With this change, can we close this issue? microsoft/WinUI-Gallery#524

@stmoy
Copy link
Contributor

stmoy commented Aug 7, 2020

Closing per the above PR

@stmoy stmoy closed this as completed Aug 7, 2020
@msft-github-bot msft-github-bot removed the needs-triage Issue needs to be triaged by the area owners label Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-TextBox TextBox, RichEditBox
Projects
None yet
Development

No branches or pull requests

5 participants