matrix-highlight
matrix-highlight copied to clipboard
Ability to join an arbitrary room and use it as a target for higlights #11
There are two TODO sections in the code that need additional discussion.
I'm also not entirely sure I handled the UI/state transitions as intended and would appreciate double-checking on that!
- upgrade matrix client minor version
- Add "watch" script for faster iteration
- Add webextension plugin to enable auto-reload in Chrome to improve iteration speed.
- Readme dev instructions update
- Fix chrome build issue (non-ascii symbols in minified version)
- Join and configure arbitrary room. Iteration 1 (works but has issues) #11
"fixes" #27 too
Thanks for your work!
I don't have time to review the whole thing today, but I am somewhat concerned about the added uses of !
. Ideally, we wouldn't be using escape hatches for the type system. If this is due to an SDK upgrade (I don't see anh other causes at first glance) I'll need to think about this some more.
Otherwise, about the WebExtension plugin: we're still at manifest V2 with this change, and thus barred from the chrome web store?
the non-null assertions are indeed because of the SDK upgrade and presumably them improving their types. I agree that ideally we should properly deal with that, but the current change doesn't change things functionally - just highlights that there is a problem we need to deal with 🤷♂️
Also, it'd be great to update to the latest major sdk version at some point!
WebExtension plugin: we're still at manifest V2 with this change, and thus barred from the chrome web store?
Yes, though it should make it easier to write a manifest that would be cross-compatible between v2 and v3 (it allows specifying browser-specific fields)
Hi, checking in on this! :)
Hey @Stvad, sorry about the delay. I really appreciate the work you're putting in. I work a full-time software job nowadays, and in all honesty I'm a little too tired most of the time to do much else software related.
Now, my thinking re: the !
and Room?
is that I knew what I was doing when I didn't check for null - I realized that the types were improperly assigned, but I was lazy and didn't bother writing the checks. Now that the checks are finally required, that little laziness debt is to be repayed. However, since you didn't introduce the possible null bugs (I did 😄 ), I think it's fine to merge the !
versions. I'll give the code another read-through now, and maybe merge if everything else looks good!
Thanks again for your help.