Comments

mpdonadio’s picture

More than likely yes, but I don't know in exactly what form, and why I don't have a branch for it yet. There are three complications.

1. I am not sure if the current architecture makes sense in Drupal 8. But, I haven't put much thought into it.

2. I am not sure where the Libraries API vs composer for contrib stands right now. I would prefer to go the composer route, but I don't know where that stands for contrib.

3. There is a open Drupal 8 issue regarding content negotiation. Conneg is needed to properly support caching when you take the User-Agent into account. The WSCCI team hasn't decided on a solution for this yet. This really need to be ironed out before a Drupal 8 port, which to do right would need to create a middleware. We talked about this last week, but no real decisions were made.

darol100’s picture

2. I am not sure where the Libraries API vs composer for contrib stands right now. I would prefer to go the composer route, but I don't know where that stands for contrib.

I think composer would be the best direction to go base on what I have read. Personally, I do not have a lot experiences with Composer.

I know is very early stages but I was playing with the Drupal Module Upgrader and this module. I have follow all the recommendation that the Drupal Module Upgrader suggested in order to make this module compatible with D8. I have done few testing and I'm not sure if the module works or not yet. I created a patch just in case someone wants to try it out. It's very experimental I'm not even sure if works or not but this is an start of the D8 version.

mpdonadio’s picture

Status: Active » Needs review

I'll branch and commit this, so there is at least a starting point.

mpdonadio’s picture

Status: Needs review » Postponed
Issue tags: +Needs reroll

This doesn't apply anymore. How about we wait until #1889824: Drupal Cache + Mobile Detect Conflict is marked as fixed and then generate a new version from that?

darol100’s picture

Sounds good to me ..

darol100’s picture

Related issues: +#2448567: Mobile Detect Demo

I'm going to wait to see if this sub-module #2448567: Mobile Detect Demo is committed or not before I started working on this.

mpdonadio’s picture

Category: Feature request » Plan
Status: Postponed » Active
Issue tags: -Needs reroll

More to come. I have plan worked out in my head.

darol100’s picture

@mpdonadio,

Responding to #1 . 2

I think we should use composer, base on conversation with many many many people that seem to be the future of php libraries in Drupal. And it seem like Libraries API is D8 direction is going to be for JS libraries.

3. There is a open Drupal 8 issue regarding content negotiation. Conneg is needed to properly support caching when you take the User-Agent into account. The WSCCI team hasn't decided on a solution for this yet. This really need to be ironed out before a Drupal 8 port, which to do right would need to create a middleware. We talked about this last week, but no real decisions were made.

Any new updates on this ?

There are two ways we can do a port.

  1. We can either use the Drupal Module Upgrader and try to do straight port, which is what I try to earlier on #2. This will make the development faster but we probably are not talking full advantage of what new tools in D8
  2. We can try to re-architect the entire project and we probably will have to open a lot issue to similar[#2398395]. This method is a little bit more complex but is going to be better.

I personally like the idea of 2, which is re-architect the entire project to see how much advantage we can get from new tools form D8.

I think that the analyzer of the Drupal Module Upgrader is a good start we probably want to run it and start fixing those issues individually on separate issues.

I'm removing the #2448567: Mobile Detect Demo issue because is a lower priority right now.

darol100’s picture

Title: D8 Support ? » [meta] Mobile Detect - Drupal 8 Port
Issue summary: View changes
Related issues: -#2448567: Mobile Detect Demo +#2615714: [mobile_detect] Mobile_detect

Changing the title into a meta issue. I have also change the description with a more generic meta issue description.

In addition, I have added the relate issue #2615714: [mobile_detect] Mobile_detect for the Drupal 8 Contrib Porting Tracker.

mpdonadio’s picture

Version: 7.x-1.x-dev » 8.x-2.x-dev

Updating version number to reflect D8, and the fact that this may not be 100% backwards compatible with 7.x-1.x.

mpdonadio’s picture

OK, I think the initial steps involve three parts

1. Wire up the module to use Compose Manager to load the Mobile_Detect library.
2. Create a service to abstract the Mobile_Detect library, probably as a thin class w/ an interface. However, I am not sure how/if you can avoid using a singleton here, which would make unit testing the service harder.
3. Create new cache contexts: a simple one for desktop/tablet/handheld, one for device type, one for OS, etc? Have to think about this.

That will be the building block for everything else. How we use the cache contexts will require some discussion.

darol100’s picture

@mpdonadio, I open 3 issues base on #11 so we can fixed them individual instead of everything at once in this big issue.

I will take care of #2627482: Integration with Composer.

darol100’s picture

Scanning into the D7 version of this project, I open few issues to that needs to be port it. Adding them as relate issue and in the summary of this meta issue.

HongPong’s picture

take a look at : https://github.com/tabbabi/detect_mobile done 2 years ago

Newb_Druper’s picture

At comment #14, I tried that link and it works but brings the same old issue that D7 version original had of page caching which has been resolved by adding more code to the settings.php.

In other words it is caching the first instance of the page and serving that same page regardless if user is on mobile or desktop. As you can see it is still an open issue https://github.com/tabbabi/detect_mobile/issues/3

Prashant.c’s picture

I tested the module created here https://github.com/tabbabi/detect_mobile only local and it seems to be working fine and detecting the device properly.

Why don't we use this as Drupal 8 version of this module ?

Prashant.c’s picture

Only issue is with the caching when used in TWIG files. Due to caching it is sometimes not providing the correct info about device type.
If this would be fixed then this would be good to go.

mpdonadio’s picture

#16, create an issue and post a patch against 8.x-2.x. I did a very quick look, and this could be a solution.

HongPong’s picture

FileSize
318 bytes

[wrong patch file , one sec]
Hi mpdonadio prashant.c, darol100, Newb_Druper, I have made a patch covering this. I set info.yml to 'layout' module package.

Most importantly this should be changed to gather the MobileDetect.php through composer. Both the source module and the library are MIT license. I added credit to tabbabi in the Readme.md which is renamed from readme.txt. I tried to catch all the machine names and reverse them to match this module. So this covers most of the necessary work.

I would assume that this would need to bust the drupal page cache for every client, which I don't see any hint of, but the author noted it does not work with regular Drupal cache.

HongPong’s picture

Status: Active » Needs work
FileSize
77.65 KB

Here is the patch. setting 'needs work' because of third party library.

mpdonadio’s picture

Status: Needs work » Active

#20, can you make a new issue with the patch? That way we can keep the review there. This is really just a Plan issue to talk about things a not for real work. Thanks.

HongPong’s picture

abhaisasidharan’s picture

Assigned: Unassigned » abhaisasidharan
FileSize
82.06 KB

Tested this patch with drupal 8.4.0. Its working fine as of now.

mpdonadio’s picture

Assigned: abhaisasidharan » Unassigned

#23, we don't need patches here; this is just a planning issue. There are various other issues for the D8 port.