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

Make selfoss navigable #869

Merged
merged 35 commits into from
Feb 12, 2017
Merged

Make selfoss navigable #869

merged 35 commits into from
Feb 12, 2017

Conversation

niol
Copy link
Collaborator

@niol niol commented Feb 6, 2017

These patches make selfoss navigable using the url hash. This makes the url reflect whats displayed in selfoss. This also makes it possible to reload the item that was open before a browser crash or when the tab containing selfoss was offloaded from memory on a mobile browser.

This is #834 rebased and with two additional fixes.

niol and others added 18 commits February 6, 2017 12:54
This pages modifies the client ui to make heavy use of location.hash: hash
changes are now responsible for loading the entry list.

Another big change is that the server side template is basically empty of any
data. It is loaded via the same client side code responsible for updating
item lists, tags, stats, etc.
The problem is explained there :
http://stackoverflow.com/questions/1703552/encoding-of-window-location-hash

As a summary, the hash exposed in JS in Firefox is already URL-encoded, which
is not the case in Chrome for instance.
- slightly better performance
- easier to read than extra explicitely requested entries for fixing fossar#774
Hash is processed asynchroneously, which would cause ajax requests to be sent
with an opened entry on slow devices. This change ensures the hash is processed
in that case.
It happened at least:
- selecting a tag containing % then changing the filter
- selecting a tag containing % then openning an item
SQLite does not support booleans as PostgreSQL does, nor has
aliases for `0` and `1` like MySQL.
@jtojnar
Copy link
Member

jtojnar commented Feb 6, 2017

Another problem:

  1. Open selfoss
  2. Click settings

Selfoss will change URL to http://localhost/selfoss/#sources/all

Which reminds me that there is no unknown route handling – if I go to http://localhost/selfoss/#newest/source-99999, it will just show empty listing. Though the API is partly to blame because it does not return an error on invalid source filter.

@jtojnar
Copy link
Member

jtojnar commented Feb 6, 2017

Okay, this fixes clicking on settings and then filters. Settings followed by tag or source are still broken.

@jtojnar
Copy link
Member

jtojnar commented Feb 6, 2017

Much better. Also, when clicking settings and then filter, no tag or source will be selected.

@jtojnar
Copy link
Member

jtojnar commented Feb 6, 2017

Looks good so far. Could you also handle non-existent sources and tags and invalid links?

@jtojnar
Copy link
Member

jtojnar commented Feb 7, 2017

I am now testing it on my live server and noticed another detail:

  1. Select “All tags”
  2. Click any other tag or source.
  3. Use browser “Back” function

The “All tags” filter will not be highlighted in the sidebar. Tags or sources do not suffer from this. The navigation works, the issue is purely visual.

@jtojnar
Copy link
Member

jtojnar commented Feb 7, 2017

Also it seems like SQLite does not support tuple comparisons. You will need to expand it as follows:

(items.datetime $ltgt :offset_from_datetime OR (items.datetime = :offset_from_datetime AND items.id $ltgt :offset_from_id))

@jtojnar
Copy link
Member

jtojnar commented Feb 8, 2017

Nice. Is it intentional that the URL is only changed when an item is clicked, not when keyboard shortcut is used?

@jtojnar
Copy link
Member

jtojnar commented Feb 8, 2017

Neat, though you still cannot navigate back/forward for these.

@jtojnar
Copy link
Member

jtojnar commented Feb 8, 2017

Additionally, is the extra_ids thing still working? When I open an item, for example, on the second page, then scroll up and reload, it is not visible on the first page. Nevertheless, the item comes up as a first item on every page after its own page.

@jtojnar
Copy link
Member

jtojnar commented Feb 8, 2017

Are not now both item id in the URL and the extra_ids completely useless? Not saying I require them, I just do not want dead code.

@jtojnar
Copy link
Member

jtojnar commented Feb 8, 2017

Thanks, now only remove the extra_ids code.

  • when the id of the item is on the first page, it will be already matched by the first part of $where_sql
  • when it is on any other page, it will be matched by the $where_ids part but discarded by the limit.

@niol
Copy link
Collaborator Author

niol commented Feb 9, 2017

For me, as said, the whole point of putting the item id in the url is that it makes it possible to reload the item that was open before a browser crash or when the tab containing selfoss was offloaded from memory. Without this, the problem was that the item was lost in read items and that it was a lot of searching to find it back, for instance to star it or to finish reading.
The extra_ids parameter is a key part in making this work.

@jtojnar
Copy link
Member

jtojnar commented Feb 9, 2017

The problem is that extra_ids only works when $where_sql is false; that is when the item is read, you are viewing unread items and there is less than maxPageLimit of them.

@jtojnar
Copy link
Member

jtojnar commented Feb 9, 2017

In other cases, because the items are sorted by datetime, id, the extra_ids items will be pushed to next pages by items with higher key.

WHERE items.source=sources.id '.$where.'
ORDER BY items.datetime '.$order.'
WHERE items.source=sources.id AND '.$where_sql.'
ORDER BY items.datetime '.$order.'
Copy link
Member

Choose a reason for hiding this comment

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

Actually they are only ordered by datetime, this should be also ordered by id for seek pagination to work properly.

@niol
Copy link
Collaborator Author

niol commented Feb 9, 2017

You're right, I'll see if I can fix that using only one query.

items.id, datetime, items.title AS title, content, unread, starred, source, thumbnail, icon, uid, link, updatetime, author, sources.title as sourcetitle, sources.tags as tags
FROM '.\F3::get('db_prefix').'items AS items, '.\F3::get('db_prefix').'sources AS sources
WHERE items.source=sources.id AND';
$order = 'ORDER BY items.datetime,items.id '.$order;
Copy link
Member

@jtojnar jtojnar Feb 10, 2017

Choose a reason for hiding this comment

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

This should be 'ORDER BY items.datetime ' . $order . ', items.id ' . $order

Otherwise, it just returns the first page over again, when you use oldest first.

@jtojnar
Copy link
Member

jtojnar commented Feb 12, 2017

If I open page at other than default filter (e.g. https://localhost/selfoss/#starred/all), selfoss.filter.type will be unread.

@jtojnar
Copy link
Member

jtojnar commented Feb 12, 2017

This looks good, if you are okay with that, I will rebase it and merge.

@niol
Copy link
Collaborator Author

niol commented Feb 12, 2017

Many thanks for your testing.
Merging should not need rebasing.

@jtojnar jtojnar merged commit cb1b2dc into fossar:master Feb 12, 2017
@niol niol deleted the ppr/navigable branch March 3, 2017 08:12
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.

2 participants