Make weblinks 4.0-compatible

joomdonation commented 3 years ago

Pull Request for Issue # .

Summary of Changes

There will be more works needed to make it finished. But at least, it is now uses 4.0 component structure and all the features works. I open it here so that we can have other users help finishing the work.

Testing Instructions

Expected result

Actual result

Documentation Changes Required

joomdonation commented 3 years ago

@Hackwar @rdeutz @HLeithner Do you know why we are receiving this error https://ci.joomla.org/joomla-extensions/weblinks/129/1/2 ?

I want to run the build script to get installable package for testing but it is not success. The error similar to:

Cannot open source directory '/tests/www/dist/current/language'

joomdonation commented 3 years ago

I include the weblink package in this PR here in case someone wants to help with testing.

pkg-weblinks-4.0.0-dev.zip

alikon commented 3 years ago

just a small issue PKG_WEBLINKS_XML_DESCRIPTION untranslated

joomdonation commented 3 years ago

@alikon Yes. Something wrong with build script. That should be fixed when the build script is fixed. For now, we can ignore that error.

alikon commented 3 years ago

sure i was just reporting my first test

alikon commented 3 years ago

my 2nd test https://github.com/joomdonation/weblinks/pull/1 :smiley:

HLeithner commented 3 years ago

drone works again thanks to @joomdonation help fixing jorobo.

Now it's time to fix the tests https://ci.joomla.org/joomla-extensions/weblinks/141/1/2

brianteeman commented 3 years ago

I've commented on some issue but mainly they are about ensuring everything moves to the namespaced stuff eg Route and not JRouter

joomdonation commented 3 years ago

Thanks @brianteeman for reviewing the changes and pointed out the issues which need to be fixed. I worked on some more clean up, these issues and more should be fixed by now. When I am less busy (this weekend), I will review everything again.

Just for information, I don't have good frontend skill, so I don't work on the layout changes. For that, I will need help from others.

As of right now, there are issues with tests (which sadly I have zero experience) and for that, I need help from @Hackwar . He is currently busy at the moment and could look at it next week (hopefully), so the work here might be delayed a bit.

joomdonation commented 3 years ago

So the current state of weblinks is that it works with Joomla 4. Below is the package in case someone wants to help with testing.

pkg-weblinks-4.0.0-dev.zip

The following items need to be done which I need help:

Review the backend layouts to make sure it is fully compatible with BS 5 and accessible
Review the frontend layouts: src/components/com_weblinks/tmpl/category/default_items.php (list weblink items from a category), src/components/com_weblinks/tmpl/weblink/default.php (weblink details page), src/components/com_weblinks/tmpl/form/edit.php (frontend submit/edit weblink form)
Null date conversion
Review and delete description attribute for fields in menu item types and in component configuration. I removed some of them but I believe there are more should be removed. I just don't know what description should be kept and which one should be removed.

chmst commented 3 years ago

I am working on layouts.

Version can be enabled in in weblinks option. The version note field is shown in the weblink form view. But the table has no version-note field.

What needs to be done: adapt the table, also with field length.

joomdonation commented 3 years ago

What needs to be done: adapt the table, also with field length.

Just did a quick check and saw that the issue happens on Joomla 3. So we should do this fix on master branch.

richard67 commented 3 years ago

@joomdonation What is the update path planned for this PR?

Shall the 4.0 version of weblinks work on 4.0 only, i.e. you first have to update the CMS core from 3.10 to 4 and then update weblinks to the new versions based on this (and other) PR's?

If that is the case (which can be done if the J3 version of weblinks being still there after the update of the CMS to 4.0 doesn't break the backend and so can be updated after that), then it could make sense to include the changes from my draft PR #416 into this one here.

joomdonation commented 3 years ago

@richard67

As of right now, my plan is having this version compatible with Joomla 4.0 only.

Since we have some bugs with PHP 8 for master branch for Joomla 3, I would expect that we fix these bugs and release new version of weblinks for Joomla 3, too. Not having experience with our release process, but I think that new version should work well on both Joomla 3.9 and 3.10.

That's what on my mind for now. So do you think that your PR should be for master branch for Joomla 3 or will it for Joomla 4 only?

richard67 commented 3 years ago

@richard67

As of right now, my plan is having this version compatible with Joomla 4.0 only.

Since we have some bugs with PHP 8 for master branch for Joomla 3, I would expect that we fix these bugs and release new version of weblinks for Joomla 3, too. Not having experience with our release process, but I think that new version should work well on both Joomla 3.9 and 3.10.

That's what on my mind for now. So do you think that your PR should be for master branch for Joomla 3 or will it for Joomla 4 only?

@joomdonation My PR is for 4 only ... check the description and the 4.0.0.sql script and you will see. My PR is doing the J4 null date stuff for weblinks, that would not work on J3.

joomdonation commented 3 years ago

Thanks @chmst for working on the layout. It looks much better now. Could you please give the following items some priority:

Weblinks detail page: Current, only few information displayed. We do not display weblink images, tags for some reasons (guess it is the same for Joomla 3). Could you please take a look at it and improve it?
Frontend submit weblink form. This need to be reviewed, too.

I just give it another round of testing and it works pretty well :).

obuisard commented 3 years ago

I am not sure how to report issues on a PR...

Found an error in administrator/com_weblinks/tmpl/weblinks/default.php line 133 replace $item->name with $name->title

If I find other issues, how should I report them? Thanks!

chmst commented 3 years ago

Thanks! You can post here - after the release you can open new issue when you click on the issues button on top

obuisard commented 3 years ago

Thanks @chmst :-)

obuisard commented 3 years ago

So far no more major errors but I have noticed:

none of the xml files use styling for fields (frontend edit as well). For instance, radio buttons should use layout="joomla.form.field.radio.switcher" rather than class="btn-group btn-group-yesno".
files from Joomla 3 version are not removed on update to Joomla 4 For instance, remove helper.php in the module
some descriptions are missing a translation For instance the module's caching descriptions are not translated (actually, should be removed to match core Joomla)
there an additional menu item type 'metadata' that should not exist.

joomdonation commented 3 years ago

@obuisard Thanks for testing and reporting the issues back. However, I have the feeling that you were using an outdated package for testing as most of the issues you reported seems to be invalid to me:

joomla.form.field.radio.switcher is used almost everywhere. The only place which we forgot to add is in the search plugin. I added it in last commit
Files are being removed during the update for component. I forgot to delete it for mod_weblinks, addressed in latest commit, too.
some descriptions are missing a translation: @chmst will help checking and fixing it if it is valid
there an additional menu item type 'metadata' that should not exist: I do not see that menu option

If it is possible, please use this updated package (which I just built from latest codebase) for testing. Many thanks

pkg-weblinks-4.0.0-dev.zip

obuisard commented 3 years ago

Sorry, I took the package available in this thread (June 26). I will try with the new package. Thank you.

joomdonation commented 3 years ago

No problem @obuisard . Thanks for testing. Please use the package which I attached here, it was built base on code from latest commit

pkg-weblinks-4.0.0-dev.zip

Thanks everyone for helping with this PR. We should now have weblinks package compatible with J4 and a bit better than J3.

obuisard commented 3 years ago

The metadata item is still present. I think it is due to metadata.xml placed in components/weblinks/tmpl/form.

metadata

ONLY when moving from 3.10 RC2 to 4.0 RC6. Not tested with final releases. On a clean 4.0 install, I do not have the issue (metadata.xml is missing in components/weblinks/tmpl/form).

obuisard commented 3 years ago

BTW great job guys :-)

joomdonation commented 3 years ago

@obuisard Thanks again for testing. The issue with the metadata menu item, I guess comes from the old package which you used for testing before (the package created on June 26)

The folder components/weblinks/tmpl/form is from our new Joomla 4 component structure (not a folder from old version of weblinks) and as you can see, we do not have that metadata.xml file anymore, so this issue won't happen with new installation or an update from Joomla 3. It comes from old weblinks package which you were used for testing only

(I tested it myself to make sure the issue does not happen on upgrade)

So if it is the only issue you are having, I think we are having a good weblinks package for Joomla 4 now :).

Attached is the weblinks package from latest codebase in case you want to test it again.

pkg-weblinks-4.0.0-dev.zip

obuisard commented 3 years ago

Thank you @joomdonation. I realized that I did not remove the old package before installing the new one. So, indeed, this was a side effect. Great job with the Weblinks release! And a big THANK YOU.

rambro commented 2 years ago

Hi I am having issues with the weblinks latest extension in J4.

In the weblinks module the weblinks module name does not appear in my joomla article.
The weblink and its description appears side by side in the joomla article. I would like the description to appear underneath the link.
_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/weblinks/436.}

joomla-extensions / weblinks