Enhance content path feature

[javerous]

Closes #2772

Changes proposed in this pull request:

• Add ability to show a whole screen pop-up which don't automatically disappear (instead of notifications).
  • /p/themes/Origine/origine.css
  • /p/scripts/main.js
  • /app/layout/layout.phtml
• Add ability to reload a whole feed and clear cache
  • /app/Controllers/feedController.php
  • /app/views/helpers/feed/update.phtml
  • /p/themes/icons/screwdriver.svg
  • /app/Models/Themes.php
• Add ability to preview content path on first article of a feed, to check if it's correct
  • /app/Controllers/feedController.php // this is probably not the right place
  • /p/scripts/extra.js
  • /app/views/helpers/feed/update.phtml
  • /p/themes/icons/look.svg
  • /app/Models/Themes.php

How to test the feature manually:

1. Use the new UI elements in feed configuration side panel.

Pull request checklist:

• clear commit messages
• code manually tested
• unit tests written (optional if too hard)
• documentation updated

Screenshots:

• The preview button next to path field (the "eye" icon) + help moved on next line

• The popup UI which preview the result of the current content-path (no need to be saved to be able to have a preview, of course)

• The new Maintenance section with 2 actions - help is perhaps redoundant, or perhaps need to be enhanced

[javerous]

For now, it's mainly a draft, to see if the core team is interested in the change. If yes, there is still a bunch of things to do (propagate translation, propagate CSS things, etc.)

• I'm not a PHP / JS developer, so my JS changes are perhaps not the best way to do what I wanted to do. Though, I tried to follow what already exists.
• There is a lot of security restriction with JS I didn't want to touch (and which I wasn't even aware it exists…) like onclick which can't be random JS anymore, apparently. It complexifies usage of JS, so as previous point, I did my best to make it work, without being (too) dirty.
• The content path preview is generated by feedController.php, but it's probably not the right place to do it. Feel free to suggest a better place for this kind of helper / tools, without having to re-implement an authentication system, like with APIs. I added some suggestions in the code, under XXX comment.
• I thought it would be a good idea to add a new Maintenance section in the feed configuration, which group a bunch of maintenances buttons. It's useful for this ticket, but I'm not 100% sure it's the best way to handle it.

About this last point, some extra thinking:

• Once someone do "Delete all articles" in the archiving section, the entries are then very slowly (i.e. after a very long moment) refetched, because of the cache, at least on my tests. So this feature by itself was not very practical to reapply content-path to articles. Plus, it forces to remove all articles already fetched by FreshRSS, and some of them are not in the remote server feeds anymore, so it's a bit unfortunate to lost articles to be able to reapply content-path.
• I wanted to add a checkbox next to the content-path field to force reapply the path to articles, but it looked a bit weird: it would have been just a temporary check box, as an extra parameter to the Submit, but for a UX point of view, it looked like a setting, which was confusing. So I thought it would be nice to have a dedicated button for that in a new "Maintenance" section: "Reload Articles". This button re-fetch everything (without removing articles already fetched), and then reapply path-content to every article in the feed. It can be time and bandwidth consuming, so perhaps this feature can be limited to admins ? At least, this was perfect for me.
• I thought it would be fine to be able to clear the cache of feed (and not only automatic clearing), so I added a button in "Maintenance". It's not specifically linked to the ticket (even if it can be used to speed up re-fetch after an "Delete all articles"), but looked nice to have this kind of feature too.

[Alkarex]

Thanks @javerous
At a quick first glance, it seems to look good 👍
I will try to test soon. Do not hesitate to post screenshots to get faster feedback from more people on the general concepts.

[Frenzie]

Not sure what I think of the icons, especially because it looks a bit like an illustration. just a textual Preview button might be preferable.

[Frenzie]

Btw, you can just paste the JS into https://jshint.com/. You don't need to install it locally or wait for the CI.

[javerous]

Not sure what I think of the icons, especially because it looks a bit like an illustration. just a textual Preview button might be preferable.

The textual button are so big… Perhaps can be a button with this icon but no text ? ^^'

What others are thinking about that ?

[javerous]

Btw, you can just paste the JS into https://jshint.com/. You don't need to install it locally or wait for the CI.

Ah yes, think you ! It will help.

[Frenzie]

The textual button are so big… Perhaps can be a button with this icon but no text ? ^^'

Note that my main point is that you should make it look like a button, like these ones:

Edit: In English

[javerous]

Note that my main point is that you should make it look like a button, like these ones:

Okay, I will try this.

[javerous]

Okay, I think that this time, it's all done ! (the tests are failing because of some time-out, I don't think it's related to my last changes…)

cc @marienfressinaud

[marienfressinaud]

I fixed some minor details I found while working on the style. The improved popup looks like this (don't pay attention to the small scrollbar glitch at the bottom of the popup, it was caused by the devtools):

I'm not totally satisfied with the controls box (maybe too grey?), but I think it's enough for now.

[javerous]

@marienfressinaud Yes, I was testing the changes right now. It's definitely better!

For the gray, I don't have special advice, it looks good to me (I'm not accustomed to see source code with white text on a dark background, but I think it's the most common).

[Edit] Oh, you were speaking about the gray of the controls box. It's perfect from my point of view.

[marienfressinaud]

I didn't style specifically the source code, just reused what was done in the rest of the application :) If we change the style here, we should change it for code in the articles.

[marienfressinaud]

Fixed for the dir="auto" 👍 Concerning the language, the same issue happens in the rest of the application. It will be at least true for the error messages and the controls ^^

[Frenzie]

Not exactly, the rest of the application has an actual application going on. :-P (But fair enough.)

[marienfressinaud]

If there's no new comments, I'll merge this PR before the end of the day :)

[Frenzie]

@marienfressinaud Like I said before, I could nitpick various things but I think it's good enough to merge. ;-)

[javerous]

🎉

[Alkarex]

Sorry for the late feedback. This is a great work :-)
One suggestion of improvement (I have not read the whole thread): If I understand correctly, the preview mode test against the Web site's home page, right?
However, the home page might not be the page on which to test the CSS path. An improvement could be to provide to either: a) expose a field on which to type an example of article URL; or b) automatically test on the latest article retrieved.

[javerous]

@Alkarex Thank you, it was a bit "laborious" (I don't know if it translates well), but mainly because I'm not a web developer ;p

One suggestion of improvement (I have not read the whole thread): If I understand correctly, the preview mode test against the Web site's home page, right?

No. I followed the suggestion from @Frenzie : I do the preview on the last article (i.e. the last retrieved) of the feed, so it should be okay (it's your "b" proposal, I think). I used the feature a bunch of time since, and it's very practical (okay, I'm perhaps not very impartial, but it helped me for sure).

A possible enhancement would be to be able to update the css path while we are inside the preview pop-up, but it seems too complex for me ;p

[Alkarex]

Great, testing more carefully on another feed indeed shows that it works as intended :-)

[Alkarex]

Slight refactoring in #4291