kfahy / slack-disable-wysiwyg-bookmarklet

Disables the WYSIWYG editor in Slack.
MIT License
824 stars 23 forks source link

Made the snippet update workspaces when you activate them #2

Closed lietu closed 4 years ago

lietu commented 4 years ago

Minor improvement: this way it works on the desktop app's developer console (which can be enabled with SLACK_DEVELOPER_MENU=true env variable), and keeps disabling the WYSIWYG as you change between teams.

kfahy commented 4 years ago

Thanks @lietu! I like the idea of this, but would prefer to not search through the experiments data every second in the background. Could we instead switch to executing the bulk of this code only if we detect a team ID change? I wonder if there's any event we could hook into to avoid polling.

lietu commented 4 years ago

Sure thing, and done. Though honestly I really can't read that shorthand syntax for JS and porting my change over to it should be done by someone else :)

Executing a couple of simple lines of JS once a second should not cause any significant amount of wasted CPU cycles in this format.

tdkn commented 4 years ago

Workspace IDs are available like this:

(() => {
  slackDebug.clientStore.workspaces
    .getAllWorkspaces()
    .forEach((workspaceId) => {
      const { redux } = slackDebug[workspaceId];

      const {
        wysiwyg_composer,
        wysiwyg_composer_ios,
        wysiwyg_composer_webapp,
        ...payload
      } = redux.getState().experiments;

      redux.dispatch({
        type: "[19] Bulk add experiment assignments to redux",
        payload
      });
    });
})();

And one question, @kfahy How did you find action type "[19] Bulk add experiment assignments to redux" ?

kfahy commented 4 years ago

Oh nice find, @tdkn! Regarding the action type, I noticed that Slack's source declared something that looked like a redux action, but with different parameters:

const m = i.b.objectOf(a.a.Experiment.isRequired).isRequired
    , f = Object(c.b)("Bulk add experiment assignments to redux");
f.meta = {
    name: "bulkAddExperiments",
    key: "experimentsStoreBulkAddExperiments",
    description: "Bulk add experiment assignments to redux"
},

I guessed that they were using a utility function to convert this object into a real one, so I tried to execute dispatches (via slackDebug.YOUR_WORKSPACE_ID.redux.dispatch) with an action type of bulkAddExperiments, but didn't get any results.

So then I overwrite the dispatch function to log the parameters, and noticed that they dispatched this [19] Bulk add experiment assignments to redux action type on load. So my guess is that they are converting the description to a type value (which seems a bit weird), prepended with a number to avoid collisions.

It's possible that this number will change with subsequent Slack client updates, but hopefully they'll implement a preference for us to disable the WYSIWYG editor before that happens!

kfahy commented 4 years ago

Also, please feel free to make a PR or extend this one with the getAllWorkspaces() approach, if we think that this is preferable to using setInterval. I could see a use case for a setInterval approach in a world where Slack tried to refresh experiment data in between loads, but I'm bullish on them adding a preference option to revert the editor before we'd have to worry about that.

Thanks for all of your feedback and contributions on this!

kfahy commented 4 years ago

I adapted this PR into a commit to update all workspaces. Thanks again for working through this.

adhocore commented 4 years ago

looks like in web browser mode, the slackDebug.clientStore.workspaces.getAllWorkspaces() call will just give one work-space (work-space in current window) even if multiple work-spaces are simultaneously logged in with the same browser

kfahy commented 4 years ago

Yeah you're right, I think this change is only relevant for the desktop app. In the browser, it looks like Slack performs a page load to switch workspaces. All of the A/B test data appears to be executed on load - while the page is loading, there's a brief period where the UI appears to have no A/B tests active.

adhocore commented 4 years ago

ok on the same note, i have simplified browser bookmarklet usage in #20 and also added a quick tip for chrome users