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.

Sign up for GitHub

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

Jump to bottom

Client side templates 3 #1216

Merged
merged 47 commits into from
Jan 26, 2021
Merged

Client side templates 3 #1216

merged 47 commits into from
Jan 26, 2021

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Sep 5, 2020 •
edited
Loading

This is the last one. After this, we can start moving setting event handlers in jQuery onto the respective components and then finally switch to react or something.

Depends on #1215
Fixes #928

@jtojnar jtojnar added this to the 2.19 milestone Sep 5, 2020
@jtojnar jtojnar marked this pull request as draft September 5, 2020 15:23
@jtojnar jtojnar force-pushed the client-side-templates-3 branch 4 times, most recently from 208534d to 2717992 Compare September 12, 2020 16:29
@jtojnar

This comment has been minimized.

Sign in to view
@jtojnar jtojnar force-pushed the client-side-templates-3 branch 2 times, most recently from b05779b to b3d8dd0 Compare September 12, 2020 16:57
@jtojnar jtojnar marked this pull request as ready for review September 12, 2020 23:05
@jtojnar
Copy link
Member Author

jtojnar commented Sep 13, 2020

I managed to reproduce the incompatibility in IndexedDB schema when upgrading:

Unhandled rejection: TypeError: item.icon is undefined

And logging out does not seem to be sufficient because selfoss.db.storage is set to null on failure so the logout cannot clear it:

https://github.com/SSilence/selfoss/blob/7bacaab321d48813777934ac129a0db1d81c5c64/assets/js/selfoss-events-navigation.js#L237

@jtojnar
Copy link
Member Author

jtojnar commented Sep 13, 2020

And looks like GCEntries is another place that expects storage to be present but it is cleared before running it:

https://github.com/SSilence/selfoss/blob/7bacaab321d48813777934ac129a0db1d81c5c64/assets/js/selfoss-db.js#L324-L330

@jtojnar jtojnar force-pushed the client-side-templates-3 branch from b3d8dd0 to 357b68c Compare September 17, 2020 16:16
@jtojnar jtojnar force-pushed the client-side-templates-3 branch from 357b68c to 10f3619 Compare October 3, 2020 08:23
@jtojnar jtojnar mentioned this pull request Oct 3, 2020
Closed
@jtojnar jtojnar force-pushed the client-side-templates-3 branch from 10f3619 to 175222c Compare October 5, 2020 20:30
@jtojnar jtojnar force-pushed the client-side-templates-3 branch from 175222c to 0e8d593 Compare October 24, 2020 18:48
@jtojnar jtojnar requested a review from niol October 24, 2020 18:48
@jtojnar jtojnar force-pushed the client-side-templates-3 branch from 0e8d593 to 271b5f1 Compare November 22, 2020 13:34
@jtojnar
Copy link
Member Author

jtojnar commented Dec 6, 2020

The clearing of selfoss.db.storage should be fixed now.

I also fixed ServiceWorker not being registered, which I broke in 16f1cd6 and sync event handler after update that I missed in dd8d428.

@jtojnar jtojnar force-pushed the client-side-templates-3 branch 4 times, most recently from 2918662 to 9ec030a Compare December 10, 2020 14:54
@jtojnar jtojnar force-pushed the client-side-templates-3 branch 2 times, most recently from 633550f to 539ffed Compare December 17, 2020 11:06
@jtojnar jtojnar force-pushed the client-side-templates-3 branch from 539ffed to e1e24e0 Compare January 4, 2021 04:33
@jtojnar
Copy link
Member Author

jtojnar commented Jan 4, 2021

I just finished porting the tag menu in the sidebar to real React – no more mutating its DOM with jQuery. We now have two JSX libraries React + react-dom here, and jsx-dom in all other templates. The latter is still used so that we can create DOM elements using JSX instead of string templating and $(html)/element.innerHTML.

I would like to get rid of DOM mutations eventually and use React everywhere but we are currently mutating the DOM all over the place. Fortunately, the conversion can be done piecemeal as demonstrated in client: Use real React for NavTags commit.

We cannot really pass new data to the components when we process HTTP responses so we need to create EventTarget objects and pass them to the components so that the components can add event listeners and we can pass them changed data through dispatching events.

@jtojnar jtojnar force-pushed the client-side-templates-3 branch from 6e8240f to f48398d Compare January 4, 2021 21:07
jtojnar added 25 commits January 26, 2021 09:47
@jtojnar
client: Move navbar toolbar to a separate component
905a977 
In preparation for removing the entire Navigation bar.
@jtojnar
client: Move Navigation sidebar to a separate component
785fa52
@jtojnar
client: Port login form to a React component
5dcaade 
This one is little uglier since I need a way of passing errors to the form.
I decided to do that by wrapping the form in a stateful component
and setting the error through its method.
@jtojnar
client: Port search list to React
702bf82
@jtojnar
client: Make eslint check subdirectories and JSX files
8642522 
Had to disable react/no-render-return-value because we rely on it and React still uses sync rendering.
@jtojnar
client: Port sources page to React
908682c 
This is almost full port of Settings/Sources page to React.

jQuery is now only used there for fadeout animation of elements that will be removed from DOM.
@jtojnar
client: Split selfoss-db file
ae9d1a0 
It is nightmare trying to edit all three objects at the same time.

No changes were made other than the split and import wrangling.
@jtojnar
client: Make _tr return the final promise
d47c0a2 
I made the change to returning the original promise in 35de644 but that makes no sense. The error handler is already re-throwing the error so `then`s would not fire if that happened and `catch`es would fire too. The only thing this does is trigger an unhandled promise rejection from the rethrown error.

Let’s revert the change and also use Promise.reject instead of throwing since throwing seems to confuse Firefox’s debugger sometimes.
@jtojnar
client: simplify NavTags reloading
d89d510
@jtojnar
client: Get rid of offset
f7ee48f 
It has not been used since the the switch to seek pagination.
@jtojnar
client: simplify offline reloadList
e91cc21 
Move filtering completely to filter method.
@jtojnar
client: Move entries ui building outside db-{on,off}line
a6b61fe 
Building the entries UI in a single place (db) will make it easier to create it with React.
@jtojnar
client: Port entries to React
f3b279c
@jtojnar
client: Port the rest of ui to React
f09099d 
No more jsx-dom
@jtojnar
client: Move shortcuts to App
9eae7a2 
Also use tinykeys so that we can reduce jQuery usage.
@jtojnar
client: Move message bar to React
6b0ff5d
@jtojnar
client: Port routing to React
405a3ec 
This will allow us to drive navigation using routes, greatly simplifying the control flow.
It will also allow us to implement long requested features like having an URL for adding sources or searching easily.

In fact search term is now part of URL.

Also the mark all as read button is now disabled on the sources page.
@jtojnar
client: Fix open button on mobile
0c910fa 
We changed it to link on desktop for better coverage but forgot to do it on mobile,
breaking it. Let’s remedy this now.
@jtojnar
news: mention react rewrite
afd23c2
@jtojnar
ci: Add react hook check to eslint
0e04b30 
As per react docs:

> We recommend using the exhaustive-deps rule as part of our eslint-plugin-react-hooks package. It warns when dependencies are specified incorrectly and suggests a fix.
@jtojnar
client: only show “no entries” after loading finishes
f273bf5
@jtojnar
client: Center the spinner
5f13c31
@jtojnar
client: Use separate spinner for load more button
7a17646 
We used to have this before.
@jtojnar
client: Scroll to top on page change
2923c90
@jtojnar
client: Clear entries on list reload
b98a3b4 
We should just remove the old list while the new one is loading to make the interface cleaner.
Also clearing a selected item is necessary to avoid navigation using shortcuts not working after switching to a different tag since the ui functions assume the selected item exists.
@jtojnar jtojnar reopened this Jan 26, 2021
@jtojnar
Copy link
Member Author

jtojnar commented Jan 26, 2021

Rebased and will merge once CI passes.

@jtojnar jtojnar merged commit b98a3b4 into master Jan 26, 2021
@jtojnar jtojnar deleted the client-side-templates-3 branch January 26, 2021 09:13
@jtojnar jtojnar mentioned this pull request Jul 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niol niol Awaiting requested review from niol

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
2.19
Development

Successfully merging this pull request may close these issues.

Switch to client-side templating
1 participant
@jtojnar