{"action":"created","issue":{"url":"https://api.github.com/repos/Virtual-TreeView/Virtual-TreeView/issues/858","repository_url":"https://api.github.com/repos/Virtual-TreeView/Virtual-TreeView","labels_url":"https://api.github.com/repos/Virtual-TreeView/Virtual-TreeView/issues/858/labels{/name}","comments_url":"https://api.github.com/repos/Virtual-TreeView/Virtual-TreeView/issues/858/comments","events_url":"https://api.github.com/repos/Virtual-TreeView/Virtual-TreeView/issues/858/events","html_url":"https://github.com/Virtual-TreeView/Virtual-TreeView/issues/858","id":393047926,"node_id":"MDU6SXNzdWUzOTMwNDc5MjY=","number":858,"title":"Fixed colors in GetColor","user":{"login":"luebbe","id":3872576,"node_id":"MDQ6VXNlcjM4NzI1NzY=","avatar_url":"https://avatars1.githubusercontent.com/u/3872576?v=4","gravatar_id":"","url":"https://api.github.com/users/luebbe","html_url":"https://github.com/luebbe","followers_url":"https://api.github.com/users/luebbe/followers","following_url":"https://api.github.com/users/luebbe/following{/other_user}","gists_url":"https://api.github.com/users/luebbe/gists{/gist_id}","starred_url":"https://api.github.com/users/luebbe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/luebbe/subscriptions","organizations_url":"https://api.github.com/users/luebbe/orgs","repos_url":"https://api.github.com/users/luebbe/repos","events_url":"https://api.github.com/users/luebbe/events{/privacy}","received_events_url":"https://api.github.com/users/luebbe/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2018-12-20T13:31:31Z","updated_at":"2018-12-28T08:03:25Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"@pyscripter @joachimmarder \r\n\r\nAfter a long pause, I have just updated VirtualTreeView to the current head revision and noticed that the inactive/disabled colors don't work properly anymore.\r\n\r\nThe patch for #789 introduced a regression. The change from clBtnFace to clGray makes `StyleServices.GetSystemColor(FColors[Index]);` always return $808080, because clGray is not a system color.\r\n\r\nYou always have to take care to choose a matching pair of text/background. If one is fixed, this is not going to work. So I suggest trying to find a pair of matching system colors.\r\n\r\nAlso clGray doesn't match the default values of the published properties, which are still clBtnFace.\r\n\r\nIn addition I think that the else case in GetColor() `Result := StyleServices.GetSystemColor(FColors[Index]);` is an oversimplification. The previous implementation, which checked whether the color was default or user defined, returned better results, at least for our VCL Styles.\r\n\r\nI will think this through and try to come up with a PR.\r\n"},"comment":{"url":"https://api.github.com/repos/Virtual-TreeView/Virtual-TreeView/issues/comments/450313018","html_url":"https://github.com/Virtual-TreeView/Virtual-TreeView/issues/858#issuecomment-450313018","issue_url":"https://api.github.com/repos/Virtual-TreeView/Virtual-TreeView/issues/858","id":450313018,"node_id":"MDEyOklzc3VlQ29tbWVudDQ1MDMxMzAxOA==","user":{"login":"luebbe","id":3872576,"node_id":"MDQ6VXNlcjM4NzI1NzY=","avatar_url":"https://avatars1.githubusercontent.com/u/3872576?v=4","gravatar_id":"","url":"https://api.github.com/users/luebbe","html_url":"https://github.com/luebbe","followers_url":"https://api.github.com/users/luebbe/followers","following_url":"https://api.github.com/users/luebbe/following{/other_user}","gists_url":"https://api.github.com/users/luebbe/gists{/gist_id}","starred_url":"https://api.github.com/users/luebbe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/luebbe/subscriptions","organizations_url":"https://api.github.com/users/luebbe/orgs","repos_url":"https://api.github.com/users/luebbe/repos","events_url":"https://api.github.com/users/luebbe/events{/privacy}","received_events_url":"https://api.github.com/users/luebbe/received_events","type":"User","site_admin":false},"created_at":"2018-12-28T08:03:25Z","updated_at":"2018-12-28T08:03:25Z","author_association":"CONTRIBUTOR","body":"> After looking at the code in some detail...\r\n> \r\n>     * Overall the changes introduced by @luebbe are useful and make the code a lot cleaner.\r\n\r\nSo we got a good starting point here :)\r\n\r\n>     * The only question is about the default values of some of the colors.\r\n> \r\n>     * UnfocusedColor is used as the font color of unfocused selected nodes, so it should probably be renamed to UnfocusedSelectionTextColor\r\nI thought the same, but wouldn't this break streaming the dfm file to/from disk? So I didn't touch it.\r\n\r\n>     * In Windows Explorer the text color of focused and unfocused selected items is the same (clHighlightText).  This is also true in the dark theme of Windows Explorer available with the Windows October update.  (by the way have you tried that?).  It should probably also be the default in VT.\r\nNo, I haven't tried that. If clHighlightText is the official default, we can change back to using SelectionTextColor instead of UnfocusedColor and don't distinguish between focused and unfocused selected node text.\r\nI had the problem that clHighlightText (white) got almost invisible on our light style which has a light gray for unfocused selection.\r\n![image](https://user-images.githubusercontent.com/3872576/50507786-3f432480-0a7f-11e9-8eb6-ad2558a0cd1c.png)\r\nUsing SelectionTextColor\r\n\r\n![image](https://user-images.githubusercontent.com/3872576/50507769-30f50880-0a7f-11e9-83ae-064fe02f2ea8.png)\r\nUsing UnfocusedSelection\r\n\r\nThis is the reason why I was looking for a different pair of contrast colors. In that case I have to talk to our designers to get a different unfocused highlight background.\r\n\r\n>     * clInactiveCaption/clInactiveCaptionText  works well in many themes including a few dark styles.\r\n> \r\n>     * However, in many dark styles clInactiveCaption is the same as clWindow  and in some clInactiveCaptionText is the same as clWindowText (eg. Windows10 Dark).  This effectively hides the selection when unfocused.\r\nOK, that means I have to add more styles to my test project. But doesn't that also mean that the VCL style in question is flawed? clInactiveCaptionText should always be readable on clInactiveCaption. Same as clWindowText should always be readable on clWindow. I guess the style designer forgot to check the SysColors before saving the Style.\r\n \r\n>     * I can get the previous appearance by setting UnfocusedSelectionColor and UnfocusedSelectionBorderColor to clGray and UnfocusedColor to clHighlightText.  This is almost by accident since clHighlightText is white in most styles. \r\nI still recommend not using fixed colors at all, because clGray will be clGray no matter if you choose a red, green or blue VCL style.\r\n\r\n> (note that in the last update StyleServices.GetSystemColor is not called for colors different to default colors - I am not sure about this @luebbe - what was the purpose of this change?).\r\nMy reasoning was that I either have a user defined color or a default color. \r\n\r\n- User defined should have precedence over everything. \r\n- A default color right now always is a system color. Calling StyleServices.GetSystemColor only makes sense if the source is a system color and style services are enabled.\r\n\r\nBut you are right. If the user chooses system colors which are different from the default, calling StyleServices.GetSystemColor still makes sense. I will revert this check.\r\n\r\nIf we revert to SelectionTextColor instead of UnfocusedColor, UnfocusedColor becomes obsolete again. I have no problem with that. Probably my case can also be remedied by adjusting the VCL style, but it may be better to have a separate text/background color pair for this case."}}
{"actor":{"display_login":"luebbe"}}