kmaterka added a comment.

  In D28107#630144 <https://phabricator.kde.org/D28107#630144>, @davidre wrote:
  
  > It seems it is possible to do this (removing stuff from the data engine) 
after all. I will try to work on this in the next time
  
  
  IMO, ideally `StatusNotifierItemSource` should just expose most of properties 
from this specification without any changes:
  
https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/
  For some complex types it should be allowed to do conversion etc.
  
  Summary of properties:
  
  | SNI property        | In specification | Simple/Complex | DataSource 
property                        | Comments                                      
                    |
  | Category            | Yes              | Simple         | Category          
                         | Direct copy                                          
             |
  | Id                  | Yes              | Simple         | Id                
                         | Direct copy                                          
             |
  | Title               | Yes              | Simple         | Title             
                         | Direct copy                                          
             |
  | Status              | Yes              | Simple         | Status            
                         | Direct copy                                          
             |
  | WindowId            | Yes              | Simple         | WindowId          
                         | Direct copy                                          
             |
  | ItemIsMenu          | Yes              | Simple         | ItemIsMenu        
                         | Direct copy                                          
             |
  | AttentionMovieName  | Yes              | Simple         | 
AttentionMovieName                         | Direct copy                        
                               |
  | ToolTip             | Yes              | Complex        | ToolTipTitle, 
ToolTipSubTitle, ToolTipIcon | Converted to separate properties, additional 
logic                |
  | Menu                | Yes              | Complex        | -                 
                         | Converted to QMenu, special handling                 
             |
  | IconThemePath       | No!              | Simple         | IconThemePath     
                         | Direct copy                                          
             |
  | IconName            | Yes              | Simple         | IconName          
                         | Only if IconPixmap is empty                          
             |
  | IconPixmap          | Yes              | Complex        | Not available     
                         | Used as part of Icon                                 
             |
  | OverlayIconName     | Yes              | Simple         | OverlayIconName   
                         | Only if OverlayIconPixmap is empty                   
             |
  | OverlayIconPixmap   | Yes              | Complex        | Not available     
                         | Used as part of Icon                                 
             |
  | -                   | -                | Simple         | Icon              
                         | Complicated logic to create it from all Icon 
properties           |
  | AttentionIconName   | Yes              | Simple         | AttentionIconName 
                         | Only if AttentionIconPixmap is empty                 
             |
  | AttentionIconPixmap | Yes              | Not available  | Used as part of 
AttentionIcon              |                                                    
               |
  | -                   | -                | Simple         | AttentionIcon     
                         | Complicated logic to create it from both 
AttentionIcon properties |
  |
  
  I skipped: IconsChanged, StatusChanged, TitleChanged, ToolTipChanged, these 
are not relevant (and not used anymore).
  
  I would suggest to:
  
  - In `StatusNotifierItemSource`:
    - Always set IconName, OverlayIconName, AttentionIconName, regardless if 
IconPixmap is set or not
    - Add new properties for all pixmaps, convert them to proper type (there is 
function for that: `imageVectorToPixmap()`)
  - Icon logic should be in `StatusNotifierItem.qml`
    - Ignore "Icon" property
    - Implement similar logic in QML, everything is available in 
`PlasmaCore.IconItem`
    - Use icon name first, then pixmap (align to specification: "Visualizations 
are encouraged to prefer icon names over icon pixmaps if both are available")
  
  Then, is a separate diff, remove unused properties/logic from 
StatusNotifierItemSource (in Plasma 6?).
  
  This ensures backward compatibility. I'm pretty sure 
`StatusNotifierItemSource` is not used outside of Plasma, but if it this is 
part of the public API then needs to stay for now.
  Presentation logic is moved to correct layer. That should simplify it a 
little bit, I had hard time understanding it for the first time.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D28107

To: davidre, kmaterka, #plasma, broulik, davidedmundson
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

Reply via email to