JustCarmen / webtrees-simple-media-display

Restore media display functionality as in webtrees 2.0. Show multiple media items and additional notes belonging to the same media object.
GNU General Public License v3.0
4 stars 2 forks source link

Hard-coded folder name #1

Closed ric2016 closed 2 years ago

ric2016 commented 2 years ago

Note that it's safer to lookup a module by interface (module_service->findByInterface(...)) than by internal name, as you do here. If a user renames the module folder, your solution won't work.

You could also just register the specific view 'jc-media-display' under the main namespace (in the module's boot() method, just like the main view) - then you don't even need the reference to the module in the main view.

JustCarmen commented 2 years ago

You could also just register the specific view 'jc-media-display' under the main namespace (in the module's boot() method, just like the main view) - then you don't even need the reference to the module in the main view.

I tried that, but than I got a variable not found error (xref, which is not set at the time of the boot yet), so I decided to implement your first suggestion. Thanks for that.

JustCarmen commented 2 years ago

I thought it was solved but my solution didn't work. I have tried different things but I couldn't get it to work without the hard coded foldername. So I decided to revert my initial commit. Maybe you can help me with this?

Btw, I will draft a new release to get rid of the error. We can solve this one later.

ric2016 commented 2 years ago

findByInterface(MediaDisplay::class) looks for any module that actually either has or implements this class 'MediaDisplay'.

So you have to adjust your module, which currently is an anonymous class.

Either use a 'normal' class named MediaDisplay (make sure to use the proper namespace when importing this in the view), similar to this module.

Alternatively, it should also work if you define an (empty) interface MediaDisplay, and let your module implement this, like so:

return new class extends AbstractModule implements ModuleCustomInterface, MediaDisplay

JustCarmen commented 2 years ago

Thanks, I was stuck but didn't realize I used an anonymous class...

JustCarmen commented 2 years ago

I thought it would be easy by now, but I still can't get it done. Even if I create a named class I can't call the internal name from the view. It's just an empty variable. When you register a custom view to override a standard view you can't add your own variables, so I need a way to call the module name from the first view, but no matter what I try it doesn't work.

The only solution I see is to not have a second view. But I prefer to keep them separate to be prepared for later updates to the default webtrees view.

I think I've spent more time trying to fix this problem than writing the whole module, so I think I'll leave it for now unless you do a pull request with a working solution.

Nevertheless, thanks for your help.

ric2016 commented 2 years ago

I thought it would be easy by now, but I still can't get it done. Even if I create a named class I can't call the internal name from the view. It's just an empty variable.

That probably indicates moduleService didn't find the module by interface. Did you set the namespace properly in the view, i.e.

use JustCarmen\Webtrees\Module\MediaDisplay?

JustCarmen commented 2 years ago

The module is found, that is not the problem. I can call for example $module->title() and other functions from the module. I just can't call $module->name().

JustCarmen commented 2 years ago

I've made a temporary branch so you can see what I mean.

ric2016 commented 2 years ago

The module is found, that is not the problem.

It isn't found via moduleService though: using app(MediaDisplayModule::class) just creates a new instance of the module, where the name isn't set.

You have to use $moduleService->findByInterface(MediaDisplayModule::class) in order to obtain the instance where webtrees has actually set the internal name.

JustCarmen commented 2 years ago

Hèhè found it. The complete code is: app(ModuleService::class)->findByInterface(MediaDisplayModule::class)->first();

The missing link was the last part (first()).