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

User queries #498

Merged
merged 5 commits into from
Jun 14, 2014
Merged

User queries #498

merged 5 commits into from
Jun 14, 2014

Conversation

aledeg
Copy link
Member

@aledeg aledeg commented May 4, 2014

It's an intermediary step to remove the favorite button.
I add a button to store the current query as a favorite query. It redirects automatically to the configuration page where it is possible to name and remove user queries.
To make the queries more straightforward, I removed the default behavior when searching for a string. This way, when we search for a string, the filter is not defaulted to all articles.

See #376

It's an intermediary step to remove the favorite button.
I add a button to store the current query as a favorite query. It redirects automatically to the configuration page where it is possible to name and remove user queries.
To make the queries more straigtforward, I removed the default behavior when searching for a string. This way, when we search for a string, the filter is not defaulted to all articles.
@Alkarex
Copy link
Member

Alkarex commented May 13, 2014

J'essayerai de tester de ces jours-ci

public function queriesAction () {
if (Minz_Request::isPost ()) {
$params = Minz_Request::params();
$this->view->conf->_queries ($params['queries']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no user query, then $params['queries'] is undefined

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you'll save an empty array.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the current code generates a warning.
You need something like:
$this->view->conf->_queries (isset($params['queries']) ? $params['queries'] : array());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird because I don't have any warning. That's why I kept the code that way.

@aledeg
Copy link
Member Author

aledeg commented May 18, 2014

Sorry for that messy PR. I forgot to watch the logs.
I hope everything is fixed now.

@marienfressinaud
Copy link
Member

Wow! That's really interesting ! :D I will continue your work if you want: there is some part of code I want to modify and I would like to improve a bit the interface ;)

@aledeg
Copy link
Member Author

aledeg commented Jun 13, 2014

Thanks.
It is in raw form because I wanted to see if there was some interest for that before going further. In my mind, instead of having a list of saved search, I wanted to have a saved search category with a drop down list of search. With maybe some options to update and/or delete them directly.
But sure, go nuts on that :)

@marienfressinaud
Copy link
Member

I have done a lot of work tonight. I will commit tomorrow because I need to sleep before check my code ;)

In few words:

  • Refactor some part of code
  • Better design in the configuration page
  • Query button has moved near of other filter buttons and becomes a dropdown menu which lists all available queries and provides a last link to add current page as a query.

@marienfressinaud marienfressinaud merged commit a611800 into FreshRSS:dev Jun 14, 2014
marienfressinaud added a commit that referenced this pull request Jun 14, 2014
- Coding style
- More checks server side
- Default query name is "Query n°X"
- List of queries is moved into nav_menu, in a dropdown
- Better system to remove fields in JS (to a.remove elements, give an
  attibute data-remove="id_to_remove")
- Fix a bug in lib/Mine/Request.php (htmlspecialchars_utf8 can be applied on
  arrays now)
- Few theme improvements
- Add an element .no-mobile to apply to elements which should not appear on
  mobiles

See #498
@marienfressinaud
Copy link
Member

It is merged! Hope you will like my improvements :)

A tip: do more work server side, default query name was given only in the view. It is better to do that on the server.

@aledeg aledeg deleted the user-queries branch June 14, 2014 10:33
@aledeg
Copy link
Member Author

aledeg commented Jun 14, 2014

Good job 👍
Maybe now is the time to remove the favorite pseudo category.

@marienfressinaud
Copy link
Member

I was thinking about that yesterday. Filters cannot replace this button for two reasons:

  1. Button shows directly how many articles are starred
  2. This button regroups all feeds, even those which don't appear in the Main stream

@Alkarex
Copy link
Member

Alkarex commented Jun 14, 2014

I can also better like to keep the existing "favourites" buttons.
Regarding the new button for user queries, I think its style should reflect the fact that it is a menu, not a radio button unlike the 4 other buttons just to its left.

marienfressinaud added a commit that referenced this pull request Jun 14, 2014
- Menu button is a down arrow
- Bookmark icon has moved near of "add a query"

See #498
@marienfressinaud
Copy link
Member

Done :)

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.

3 participants