When using black color themes, imported data is unreadable. Table background is white, but text color is taken from color theme as a result, it is almost unreadable. Please see attached file for example. Reproducible: Always Expected Results: Colors should be taken from system color pallet so that contrast between foreground and background is always present.
Created attachment 90243 [details] Example of unreadable text
Created attachment 90244 [details] Proposed patch Proposed patch: - colors are read from KColorScheme, - different color for indicating ignored lines and errors, - indication of ignored lines is made by setting flags and colors of cells to disabled state, - selection ability for table is turned off (it is not needed for anything and can create a mess when user click on a cell), - minor issue fixed - column with decimal point was resetting background in ignored lines - minor code cleanup in CSVDialog::updateDecimalSymbol to improve readability.
Created attachment 90245 [details] Example for black theme with patch
Created attachment 90246 [details] Example for bright theme with patch
Hi Jakub Many thanks for your interest and suggestions, and especially for the patch. Patches are always welcome. I'll come back to this in a few days, as I just want to finish off what I've been working on.
Hi Jakub, I took a look at the patch. I glanced through it and it looked ok, but it affects several issues so it would be much easier if you can split the patch. That way, each fix can be reviewed independently. Some will be probably accepted very quickly and some require a review by a more specific maintainer. Thank you for your contribution.
Thanks a lot for looking at it. You are right, I will split this patch to seperate use cases. It will be easier to handle since apparently allan has changes in ui stacked up.
On 06/01/15 15:34, Jakub Grandys wrote: > https://bugs.kde.org/show_bug.cgi?id=342535 > > --- Comment #7 from Jakub Grandys <me@pennguin.net> --- > Thanks a lot for looking at it. > You are right, I will split this patch to seperate use cases. It will be easier > to handle since apparently allan has changes in ui stacked up. > Yes, there is a fairly drastic change to the UI in line, so it might might be worth hanging on a bit, to avoid everything/parts needing reworking yet again. Allan
Created attachment 90354 [details] Patch with only color changes
I'm sorry for delay, nevertheless I've attached new version of patch - only hardcoded colors are replaced with calls to KColorScheme. One addition is to set also foreground - without it and default KDE's color scheme, markers are not so easily visible. I will open new feature request bug against csvimporter with additional patches for marking rows as disabled instead using disabled colors plus some fixes in handling those lines (the same as in first version). Is it ok, with you guys?
Created attachment 90365 [details] attachment-13663-0.html Yes, that's really good. Thank you for your continuous interest. El dom, ene 11, 2015 13:33, Jakub Grandys <me@pennguin.net> escribió: > https://bugs.kde.org/show_bug.cgi?id=342535 > > --- Comment #10 from Jakub Grandys <me@pennguin.net> --- > I'm sorry for delay, nevertheless I've attached new version of patch - only > hardcoded colors are replaced with calls to KColorScheme. One addition is > to > set also foreground - without it and default KDE's color scheme, markers > are > not so easily visible. > > I will open new feature request bug against csvimporter with additional > patches > for marking rows as disabled instead using disabled colors plus some fixes > in > handling those lines (the same as in first version). Is it ok, with you > guys? > > -- > You are receiving this mail because: > You are the assignee for the bug. > _______________________________________________ > KMyMoney-devel mailing list > KMyMoney-devel@kde.org > https://mail.kde.org/mailman/listinfo/kmymoney-devel >
I've installed that without problem. Are you OK for it to go ahead?
I'm ok to push the color fixing patch.
Git commit 5dfef05b5a81233d06e4777477204f91dee9e5cf by Allan Anderson. Committed on 13/01/2015 at 13:37. Pushed by allananderson into branch 'master'. Fix use of hardcoded colors inside preview table of CSVImporter. With thanks to Jakub Grandys. M +17 -10 kmymoney/plugins/csvimport/csvdialog.cpp M +3 -3 kmymoney/plugins/csvimport/csvdialog.h M +0 -37 kmymoney/plugins/csvimport/csvdialog.ui http://commits.kde.org/kmymoney/5dfef05b5a81233d06e4777477204f91dee9e5cf
(In reply to Alvaro Soliverez from comment #13) > I'm ok to push the color fixing patch. Right. The same issue exists in redefinedlg.cpp, but I'll just push the initial patch for now. (In reply to Jakub Grandys from comment #1) > Created attachment 90243 [details] > Example of unreadable text Could you say what theme you used to produce the unreadable example, so I can test. The dark examples in the settings don't show a problem, unless I'm missing something.
Thank you, I've checked latest master - it is working :) (In reply to allan from comment #15) > Could you say what theme you used to produce the unreadable example, so I > can test. The dark examples in the settings don't show a problem, unless > I'm missing something. It is "Obsidian Coast" - should be default in KDE - had the same issue on two machines. In case you are missing it, I've attached it to bug.
Created attachment 90391 [details] Dark color theme Color theme which was having issues.
(In reply to Jakub Grandys from comment #17) > Created attachment 90391 [details] > Dark color theme > > Color theme which was having issues. OK, thanks for that. Obsidian Coast is in the standard settings. A few observations. In the CSV plugin, there are the expected improvements, except that a few issues remain - the file selector, the combobox drop-downs and a couple of the smaller windows. Also, there is a more general problem to do with KMyMoney itself, for instance the Home screen, the Views panel and the Ledger window, where only entries in red are legible. I'm not addressing this to you, Jakub, but to the whole team. I think you've let the genie out of the bottle. Thanks for all that! Allan
Just to summarise, there is a general problem in KMyMoney affecting certain views, file selectors and combobox dropdowns, but apparently only with the Obsidian Coast colour scheme I think. Other dark schemes appear to be alright. I don't therefore see any point in looking further into the CSV importer. There is just one cell in one little-used window that still uses a hard-coded colour. The patch that's been applied helps in some respects with the Obsidian Coast colour scheme, but the more general points mentioned above remain here too.