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

Incompatible with Nextcloud v25 #1945

Closed
Grotax opened this issue Oct 21, 2022 · 13 comments · Fixed by #1950
Closed

Incompatible with Nextcloud v25 #1945

Grotax opened this issue Oct 21, 2022 · 13 comments · Fixed by #1950
Labels
bug frontend impact Javascript/Frontend code help wanted

Comments

@Grotax
Copy link
Member

Grotax commented Oct 21, 2022

Why

News is not compatible with Nextcloud v25 and we haven't released a new version for it.

The issue is not the backend code written in PHP, which is actually working fine on Nextcloud 25 but the frontend code written in JS, to be precise in the original angular js which is no longer supported.
This has been an issue for quite some time (since 2017) see #195

With Nextcloud 25 the server Team decided to rebuild parts of the Nextcloud frontend which resulted in lots of improvements.
But has made News partly incompatible.

Also for a longer time there is a move towards vue js within the community and there is a repository with standard components that can be used in all apps. https://github.com/nextcloud/nextcloud-vue

Issues

Issues you might encounter are:

Solutions

You may force enable the app, if you are mainly using an app that communicates via the API you should be ok.
In occ you can do this:

php ./occ app:enable --force news

The floating title bar can be fixed by the changes in #1944

The actual long term solution would be a re-write of the UI with vue-js.
Original PR: #748
Branch: https://github.com/nextcloud/news/tree/vue-rewrite
Current Work in Progress: https://github.com/devlinjunker/news/pulls

Second attempt: #1886

Feel free to support any of these to enhance the situation.

@Grotax Grotax added the bug label Oct 21, 2022
@Grotax Grotax pinned this issue Oct 21, 2022
@Grotax Grotax added frontend impact Javascript/Frontend code help wanted labels Oct 21, 2022
@rhyst
Copy link
Contributor

rhyst commented Oct 21, 2022

Can I just confirm that you would potentially accept my PR to fix the existing frontend on NC25?

I'm worried I'm missing something about the extent of the "incompatibility" as for me it seems to work perfectly on my PR and I only fixed two minor issues.

Also I believe I have fixed #1932 as well in my PR.

@Grotax
Copy link
Member Author

Grotax commented Oct 21, 2022

I haven't looked at it yet in detail. Question is of course if this also works with older nextcloud versions. We could of course limit comparability to 25+ but that would mean no more updates for the older NC versions, as we do not provide updates to older branches.

@alerque
Copy link

alerque commented Oct 21, 2022

All the best to those contributing and testing this.

Just a heads up to say that this is blocking Arch Linux from updating Nextcloud to v25 in official repos. We're following along and will be able to bump Nextcloud (and all the matching official app packages we have) as soon as a version of this lands that declares v25 as supported.

For Arch distro purposes we don't care about continuing to support v24 asa we only support currently packaged releases of everything, but I understand that testing and confirming that is probably a valid concern for other use cases.

@rhyst
Copy link
Contributor

rhyst commented Oct 21, 2022

I have made the fixes conditional on NC25 so it shouldn't affect previous versions but I have not tested it yet. I wanted to confirm that there was interest in merging such a PR before I invest any more time in it.

I agree that a modern Vue rewrite is the way to go long term but given how minor the fixes are that are needed it seems like it would be beneficial to release something sooner.

@derritter88
Copy link
Contributor

My personal opinion would also be that News should be supporting the newest Nextcloud version (PR of @rhyst).
Any older versions are mostly used by Enterprise license users.

Unfortunately my JS programming knowledge is very limited as I was a Java developer long time ago.
But I would be happy to help you in any way you might figure out (e.g. alpha/beta testing)

@Grotax
Copy link
Member Author

Grotax commented Oct 22, 2022

Yea we are working on it, it takes time though to test these changes. Since we do not have trustworthy automated ui testing the only way to do this is to build the app and install it in a nextcloud instance then check manually.
Feel free to do so an leave feedback on the PR.

@derritter88
Copy link
Contributor

Yea we are working on it, it takes time though to test these changes. Since we do not have trustworthy automated ui testing the only way to do this is to build the app and install it in a nextcloud instance then check manually. Feel free to do so an leave feedback on the PR.

Is there anything I need to keep in mind (e.g. like building the app) or is it sufficient to uninstall the regular "News" app within Nextcloud, git clone the repo and edit the PR and install it manually within Nextcloud?

I recoginzed that there are some JS files missing in the regular News app at the current Nextcloud installation.

@Grotax
Copy link
Member Author

Grotax commented Oct 22, 2022

You need to run make within the the directory of apps/news.

That will require composer (which will be installed automatically I think) and npm with all it's dependencies.

After that the app should be usable.

@Grotax Grotax linked a pull request Oct 22, 2022 that will close this issue
@derritter88
Copy link
Contributor

Okay done that (after playing around with dependencies and NodeJS version of Ubuntu 22.04).

So far it looks good -Thanks to @rhyst !

@rhyst : It works that articles are being marked as read when you scroll through them, but if you use cursor keys they will be still unread.
Not sure if this is also possible to fix that?

@rhyst
Copy link
Contributor

rhyst commented Oct 22, 2022

@derritter88 I had a play around and tried to fix what I noticed here: #1953

Happy to update it if there was something else not working that you found.

@derritter88
Copy link
Contributor

I can confirm, that #1953 has fixed the issue with the Keyboard.

From my point of view the 19.0.0.-beta2 version is working very stable.
I do not see any errors directly at the News app and also not at the Admin logging overview.
(Mobile app/API is also working stable.)

@derritter88
Copy link
Contributor

Hello everyone,

just wanted to mention that 19.0.0-beta2 and the current 19.0.0 version are running stable without any issues.

@joopdo
Copy link

joopdo commented Oct 26, 2022

Same here! Thanks to all those involved.

@SMillerDev SMillerDev unpinned this issue Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug frontend impact Javascript/Frontend code help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants