App: improve pinned_action state

[danirabbit]

Fixes #520

Replaces SimpleAction with PropertyAction

[leolost2605]

Maybe we should make the pinned action a property action?

[danirabbit]

@leolost2605 yeah I was thinking about that but I wasn't sure how to handle that we construct with it's initial value? Feel free to take over 😅

[danirabbit]

@leolost2605 I really want to get this in because I think this is a big part of #520

[leolost2605]

We can merge as is if you want because I can't promise I have the time to get this done today or tomorrow. Did you confirm if this fixes #520 and if so did you have time to figure out how? I'm asking because I wonder what is happening because what's going on on main is

pinned_action.change_state 
  => pinned = new_state
    => notify pinned handler
      => pinned_action.set_state
      => check_remove
      => sync_pinned

and with this PR it's

pinned_action.change_state 
  => pinned_action.set_state
  => check_remove
  => sync_pinned
  => pinned = new_state
    => notify pinned handler
      => pinned_action.set_state // Not sure if this is intended?

There are two things I noticed

1. we did call set_state in the change state handler on main as well just not in the handler itself but a method called by it but that shouldn't make a difference?
2. with this PR sync_pinned is called before the pinned property is actually updated so it doesn't really do anything anymore? If that is what fixed #520 maybe we should look elsewhere because that's not really expected behavior?

I don't have the time today to do some testing or more investigation so I might've easily missed something it's just that I noticed that and was a bit confused :)

[danirabbit]

@leolost2605 oops thanks for catching that I was calling sync before setting the variable. Changed that

I tested by:

1. pinning an item with the context menu (using the keyboard because of Dock menus don't accept clicks when launched by Gala gala#2667)
2. killing the dock (killall io.elementary.dock)
3. and then when it restarted the item was still pinned

and then I did the inverse, unpinning and killing and making sure it was still unpinned

@LuminousHustler do you have time to test and confirm this branch works for you?

[danirabbit]

@leolost2605 I updated this to use PropertyAction. I wasn't aware of this subclass so I thought you were asking me to make the SimpleAction a property 😅 This is way cleaner, thanks for the suggestion!

[leolost2605]

Ahh 😅
But the problem with this is - and the main reason why I changed so much in #521 apart from it being better architecture IMO - that like this it forms a reference cycle because
app -> action group -> property action -> app :(