Fully working again?#532
Conversation
|
Seems like it's not entirely working… 😉 |
|
(Also, if you felt like taking over maintainership, I'd be more than happy to pass it along.) |
|
CI is passing now 👍 |
|
Curious: does this fix the issue where you can't snooze except by right-clicking on an inactive tab and choosing one of the default options? If so, ❤️. |
|
It seems to fix everything... feel free to test |
| }, packageMeta.webextensionManifest); | ||
| const manifest = Object.assign( | ||
| { | ||
| name: 'SnoozeTabs', |
There was a problem hiding this comment.
Given everything else in this block, what do you think about making this packageMeta.display_name and adding that to the package.json?
| "GA_TRACKING_ID": "UA-90911916-2", | ||
| "productionLocales": "en-US,ar,bn-BD,bs,cak,cs,de,dsb,el,es-AR,es-CL,es-ES,es-MX,fa,fr,fy-NL,he,hsb,hu,id,it,ja,ka,kab,ms,nb-NO,nl,nn-NO,pt-BR,pt-PT,ro,ru,sk,sl,sq,sr,sv-SE,te,tr,uk,zh-CN,zh-TW" | ||
| }, | ||
| "webextensionManifest": { |
There was a problem hiding this comment.
Is there a reason we moved this out of here and into the generateManifest.js file?
| "bundle:css": "npm run lint:sass && shx mkdir -p dist/popup && node-sass src/popup/snooze.scss > dist/popup/snooze.css", | ||
| "package": "cross-env NODE_ENV=production npm run build && web-ext build -s dist --overwrite-dest && mv web-ext-artifacts/*.zip addon.xpi", | ||
| "package:dev": "cross-env NODE_ENV=development npm run build && web-ext build -s dist --overwrite-dest && mv web-ext-artifacts/*.zip addon-dev.xpi", | ||
| "bundle:js": "NODE_OPTIONS=--openssl-legacy-provider npm run lint:js && NODE_OPTIONS=--openssl-legacy-provider webpack", |
There was a problem hiding this comment.
This seems like a lot of duplication. Can we set it once in a more common place, and let it be picked up from there?
| }, | ||
| "engines": { | ||
| "node": ">=8.9.4" | ||
| "node": ">=16.0.0" |
There was a problem hiding this comment.
Could we make this node 18, to match the circleci config?
| "contributors": [ | ||
| "Les Orchard <me@lmorchard.com> (http://lmorchard.com/)" | ||
| ], | ||
| "contributors": ["Les Orchard <me@lmorchard.com> (http://lmorchard.com/)"], |
There was a problem hiding this comment.
At this point, maybe you should add yourself to the list of contributors… 😉
| function flashFavicon(tab) { | ||
| // FIX 3: executeScript on a freshly-created tab can fire before the page is | ||
| // ready, producing "Actor 'Conduits' destroyed" rejections. We catch | ||
| // and log rather than letting them become unhandled. |
There was a problem hiding this comment.
Can we wait for the page to be ready instead?
| return messageOps.confirm(message); | ||
| } | ||
|
|
||
| // FIX 1: Use browser.tabs.sendMessage (not chrome.tabs.sendMessage). |
There was a problem hiding this comment.
We should remove the comments that have already been implemented.
| @import 'node_modules/rc-calendar/assets/index'; | ||
| @import 'node_modules/rc-time-picker/assets/index'; | ||
| @import 'node_modules/photon-colors/colors.scss'; | ||
| @import '../../node_modules/rc-calendar/assets/index'; |
| 'message': message | ||
| }); | ||
| sendShowUpdate(showCheckbox); | ||
| const bar = document.createElement('div'); |
There was a problem hiding this comment.
I'd prefer to leave this as confirmationBar to indicate what it's for…
|
In all honesty - I didn't make ANY of the changes that fixed the add-on - Opencode did - I was just the monkey testing the add-on in the browser and giving feedback. I always wanted SnoozeTabs to work again, and also wanted to test out the capabilities of Opencode... so that's what I did. I'm super busy with my day-to-day work right now and unfortunately don't really have much time for more on this. I did scan over your review comments though, and they do seem sensible. |
3 participants
I had OpenCode have a bash at getting it all working again... seems to have worked!
Note: I had to
npm install --legacy-peer-deps