Impact Analysis of generic darkmode rendering of plugin dock panels (from PR #11711 )

[Shridhar Kumar]

[ This is feedback for some of the points discussed in Notepad++ PR #11711. So, the referred quotes in this post refer to the conversation items in that PR. ]

@donho & @ozone10:

Let’s remove (comment) it. We will see users’ reaction about this feature to add it back, or to add an non-GUI option in config, or even an empty xml file as sentinel for enable this option.

I have tested the NPP commit with my Fixed-Width Data Visualizer plugin and providing the relevant feedback here.

Darkmode support was added to the FWDataViz plugin soon after it featured in NPP, over a year ago. Now with the generic darkmode rendering for all plugin dock panels in NPP, I was interested to see if this caused issues with FWDataViz which already had its own darkmode support. My observations:

1. Thankfully, there were no crashes or interface blemishes when NPP’s generic darkmode rendering and the plugin’s own darkmode rendering were both simultaneously active.

2. Also, the plugin panel was getting the darkmode enable/disable toggles and tone changes immediately, with no restart required in NPP. This could be because maybe I missed turning off all the darkmode render code in FWDataViz, OR I am misunderstanding this statement:

Also current implementation require restart, user still must close Notepad++ to apply new config.

3. When I had just the NPP darkmode rendering active, it failed to do its job only on the three-state checkbox on the FWDataViz side panel.
   
   That can be easily fixed, if needed, in the NPP darkmode render code by accounting for BS_AUTO3STATE controls as seen in these code snippets from the adapted version of NppDarkmode.cpp for the FWDataViz plugin:

...
         if (wcscmp(className, WC_BUTTON) == 0)
         {
            auto nButtonStyle = ::GetWindowLongPtr(hwnd, GWL_STYLE) & 0xF;
            switch (nButtonStyle)
            {
               case BS_CHECKBOX:
               case BS_AUTOCHECKBOX:
               case BS_AUTO3STATE:
               case BS_RADIOBUTTON:
               case BS_AUTORADIOBUTTON:
                  if (p.subclass)
                  {
                     subclassButtonControl(hwnd);
                  }
                  break;
...
...
...
      int iPartID = BP_CHECKBOX;
      if (nButtonStyle == BS_CHECKBOX || nButtonStyle == BS_AUTOCHECKBOX || nButtonStyle == BS_AUTO3STATE)
      {
         iPartID = BP_CHECKBOX;
      }
...

4. The generic darkmode render code also doesn’t support any other popup dialogs that are opened by the FWDataViz plugin. This clip was taken by turning off FWDataViz’s own darkmode rendering, and with just NPP’s generic darkmode rendering in action.
   

5. In view of the preceding observation and to avoid the needless double rendering of darkmode for the plugin, I had to add the DWS_USEOWNDARKMODE flag to FWDataViz’s dock panel creation code to turn-off the generic darkmode rendering from NPP, and turn on the FWDataViz’s custom darkmode rendering.
   

The changes made to FWDataViz code along with other issue fixes are already available in the latest FWDataViz 2.5.2.0 release.

[donho]

@shridhar-kumar said in Impact Analysis of generic darkmode rendering of plugin dock panels (from PR #11711 ):

In view of the preceding observation and to avoid the needless double rendering of darkmode for the plugin, I had to add the DWS_USEOWNDARKMODE flag to FWDataViz’s dock panel creation code to turn-off the generic darkmode rendering from NPP, and turn on the FWDataViz’s custom darkmode rendering.

Thank you for your info, very appreciated.

So currently, you use DWS_USEOWNDARKMODE to avoid dark mode processing from Notepad++ and render dark mode by you own, and there’s no more issue in v2.5.2.0 release. Could you confirm it?

Have you tested v2.5.2.0 with Notepad++ v8.4.2 or v8.3.3 ?

[Shridhar Kumar]

@donho,

Yes, my plugin is working well with both NPP v8.4.2 and the NPP build from the commit with #11711 PR.

Prior to NPP 8.4.1, my plugin was providing darkmode support by querying the config.xml. This involved linking to TinyXML library. With the release of NPP 8.4.1, the plugin only relies on the darkmode query APIs that were added in NPP. Subsequently, I removed the code for querying config.xml and the TinyXML linking. So, the current version of FWDataViz only works with NPP 8.4.1 and higher versions.

[Shridhar Kumar]

To clarify further, having the DWS_USEOWNDARKMODE flag added in the FWDataViz 2.5.2.0 release is not causing any issues at all when it is used either in NPP 8.4.1 or NPP 8.4.2 (i.e., when those NPP versions aren’t processing/handling the DWS_USEOWNDARKMODE flag).

In NPP 8.4.0 or NPP 8.3.3, the 2.5.2.0 version still works but without any darkmode support because of the reasons mentioned in the preceding post.

[donho]

@shridhar-kumar

Nice to learn that DWS_USEOWNDARKMODE does the trick.
Thank you for the very detailed information!

[Michael Vincent]

@donho

There are some side effects from this commit on Plugins when Dark Mode is DIS-abled. I don’t use Dark Mode, and observed the behavior of plugins changes with this commit even when Dark Mode is not enabled. I commented on the commit:

https://github.com/notepad-plus-plus/notepad-plus-plus/compare/v8.4.2...master#commitcomment-75431005

But essentially, using Explorer plugin as example:

BEFORE:

AFTER:

Notice the “AFTER” image lower pane no longer has any file icons. The only change for this “demo” was running with Notepad++ artifact before this commit and then Notepad++ artifact from after this commit.

Cheers.

[Shridhar Kumar]

@donho, with the addition of COLORREF disabledEdge member to the NppDarkMode::Colors struct, today’s commit of Use edge colors in dark mode for listbox border to the Notepad++ repo has affected the Darkmode Query API for plugins added only recently in NPP 8.4.1.

I am looking forward to the day when all these darkmode-related changes are stabilized so that developers who are supporting darkmode in their plugins don’t have keep playing catch-up with every darkmode-related commit in Notepad++.

[Bas de Reuver]

The generic darkmode rendering looks pretty cool. 👍 I’m trying get my plugin to make use of this as well. The plugin is created using C#, but for the WinForm controls this shouldn’t matter afaik.

However, as far as I can see the function autoSubclassAndThemeChildControls iterates over the EnumChildWindows but it isn’t called recursively for container-type controls.

I mean some controls like Panel, GroupBox, SplitContainer are containers which have ChildControls, so they contain labels and buttons etc. So a textbox inside a container doesn’t get the DarkMode treatment, see screenshot below (btw I’m using the v8.4.3 RC)

I’m not familiar with C++ so I don’t know how to detect if it’s a container control. But fyi in Visual Studio all the controls shown under Containers are: Pointer, FlowLayoutPanel, GroupBox, Panel, SplitContainer, TabControl, TableLayoutPanel.
Not sure if that’s all the possible containers though…

[Bas de Reuver]

Also, what is the best way to apply DarkMode colors in other windows and dialogs? 🤔 There are other dialogs that are started from the dockable window, and also from menu items.