Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow extension to work again #4

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mathomp4
Copy link

@mathomp4 mathomp4 commented Dec 8, 2022

Closes #3

I am a Fortran programmer by training, but I managed to hack around JS or Jquery or whatever is happening here to get something to work. Probably good for @tfer6 to try it out and see if it works for them. If so, then @davidz627 can scream at what I did and do a good job. 😄

I also tried moving to Manifest v3 by looking at https://developer.chrome.com/docs/extensions/mv3/mv3-migration-checklist/#api-background-pages

I mean...it seems work and Chrome doesn't complain? 🤷🏼

@tfer6
Copy link

tfer6 commented Dec 8, 2022

I downloaded it, and loaded the unpacked extension. It appears to work fine.

Chrome did throw an error when I loaded it stating "'browser_action' requires manifest version of 2 or lower'. It pointed to these lines of code

  "browser_action": {
    "default_popup": "popup.html"
  },

Not sure if I could have caused this, or something else?

@mathomp4
Copy link
Author

mathomp4 commented Dec 8, 2022

I downloaded it, and loaded the unpacked extension. It appears to work fine.

Chrome did throw an error when I loaded it stating "'browser_action' requires manifest version of 2 or lower'. It pointed to these lines of code

  "browser_action": {
    "default_popup": "popup.html"
  },

Not sure if I could have caused this, or something else?

Hmm. I didn't see that error but let me try fixing it up...

@mathomp4
Copy link
Author

mathomp4 commented Dec 8, 2022

Yeah, per the docs, we now have action not browser_action and page_action.

I pushed a fix and it seems to still work? Not sure what that action does, so I don't know if I broke anything. :)

@tfer6
Copy link

tfer6 commented Dec 8, 2022

No error anymore and it still seems to work! Appreciate it! I still can't believe this isn't a native feature for Feedly.

@tfer6
Copy link

tfer6 commented Dec 9, 2022

Welp, I apparently wasn't paying attention last night when I was testing it. It seems to work fine on all of the "views" except article view. On article view it just opens whatever the 1st item is in the feed regardless of where you are.

After playing around with it a bit, I believe Feedly might have changed something on their end. It appears they no longer "highlight" the current article when using article view, like they do for the three other views.

@mathomp4
Copy link
Author

mathomp4 commented Dec 9, 2022

Welp, I apparently wasn't paying attention last night when I was testing it. It seems to work fine on all of the "views" except article view. On article view it just opens whatever the 1st item is in the feed regardless of where you are.

After playing around with it a bit, I believe Feedly might have changed something on their end. It appears they no longer "highlight" the current article when using article view, like they do for the three other views.

Ahh. I guess I never use that view! Let me take a look and see.

@mathomp4
Copy link
Author

mathomp4 commented Dec 9, 2022

Welp, I might be too dumb at this. I cannot figure out how to tell what the current article in "focus" is in that view. I mean, I see all sorts of jQuery posts out there about focus and such, but everything I try throws an Error!

Heck, before today I didn't even know that was a view in Feedly. I just always do Title view!

@tfer6
Copy link

tfer6 commented Dec 9, 2022

Yea, I was looking around and didn't see anything that put the current article in "focus". To be honest, I haven't changed the view on Feedly in years. There probably was a reason to use article view, however it looks like it now basically functions like title view. Good enough for me! Again, thank you!

@koenhausmans
Copy link

I've had a look as well, personally I fixed it by changing:
var selectedArticle = $('.entry--selected').find('a.entry__title')
to
var selectedArticle = $('.entry--selected').find('a.EntryTitleLink-selected')

This seems to work here under Title-Only View, Magazine View and Cards View.
Just not under Article View, but I cannot seem to find any HTML attribute being changed for the active article in this view. I couldn't find an easy fix for that view.

@mathomp4
Copy link
Author

I've had a look as well, personally I fixed it by changing: var selectedArticle = $('.entry--selected').find('a.entry__title') to var selectedArticle = $('.entry--selected').find('a.EntryTitleLink-selected')

This seems to work here under Title-Only View, Magazine View and Cards View. Just not under Article View, but I cannot seem to find any HTML attribute being changed for the active article in this view. I couldn't find an easy fix for that view.

The fact you knew enough to do that means you are better at this than I am. I had to stare at the Chrome Developer's panel thing for like an hour to try and figure out what's going on. 😄

@koenhausmans
Copy link

It appears that another change has occured at feedly. They changed the "EntryTitleLink-selected" class to an ID.

Right now this seems to work here:
var selectedArticle = $('.entry--selected').find('a#EntryTitleLink-selected')

@mathomp4
Copy link
Author

It appears that another change has occured at feedly. They changed the "EntryTitleLink-selected" class to an ID.

Right now this seems to work here: var selectedArticle = $('.entry--selected').find('a#EntryTitleLink-selected')

Dang. My "doesn't know what I'm doing" version I guess is so inelegant it doesn't even see this change. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stopped working for me today
3 participants