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

PR: feature/471 #303

Closed
wants to merge 9 commits into
base: dev
from

Conversation

Projects
None yet
2 participants
@jaswrks
Copy link
Contributor

jaswrks commented Dec 8, 2016

jaswsinc added some commits Dec 8, 2016

jaswsinc
jaswsinc
jaswsinc
Adding `thadafinser/user-agent-parser` dependency to `composer.json` …
…and removing PHP 5.5 from the Travis CI build tests, as that will fail now that we have a dependency that requires PHP 5.6. Note however that PHP 5.6 will only be required for the mobile mode feature being added.
jaswsinc
jaswsinc
@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

Disabled (default state)...

2016-12-10_10-03-33


Enabled state...

2016-12-10_10-05-26

@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

New Official Public API Methods (Pro Version Only)

These work with or without Mobile-Adaptive Mode enabled, and used outside of the Mobile-Adaptive Mode feature, they will also return data for desktops and laptops; i.e., whatever the current UA is.

  • print_r(comet_cache::uaInfo());

    /**
     * UA (User-Agent) info.
     *
     * @since 16xxxx Mobile-adaptive salt.
     *
     * @param string|null $ua User-Agent (optional).
     * @note Defaults to `$_SERVER['HTTP_USER_AGENT']`.
     *
     * @return array UA info (or empty array on failure).
     *
     * The array will contain the following keys:
     *
     * - `os.name` = iOS, Android, WinPhone10, WinPhone8.1.
     *
     * - `device.type` = Tablet, Mobile Device, Mobile Phone.
     * - `device.is_mobile` = True if a mobile device (e.g., tablet|phone).
     *
     * - `browser.name` = Safari, Mobile Safari UIWebView, Chrome, Android WebView, Firefox, Edge Mobile, IEMobile, IE, Coast.
     * - `browser.version` = 55.0, 1.3, 9383242.2392, etc.
     */
    public static function uaInfo($ua = null)
    {
        return $GLOBALS[GLOBAL_NS]->getUaInfo($ua);
    }
  • if (comet_cache::uaIsMobile()) {

    /**
     * UA (User-Agent) is mobile?
     *
     * @param string|null $ua User-Agent (optional).
     * @note Defaults to `$_SERVER['HTTP_USER_AGENT']`.
     *
     * @since 16xxxx Mobile-adaptive salt.
     *
     * @return true True if is mobile.
     */
    public static function uaIsMobile($ua = null)
    {
        return $GLOBALS[GLOBAL_NS]->uaIsMobile($ua);
    }
@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

Updated composer.json Dependencies

"require": {
    "php": ">=5.4",
    "websharks/wp-php-rv": "160824.6416",
    "websharks/html-compressor": "161208",
    "thadafinser/user-agent-parser": "1.5.0",
    "browscap/browscap-php": "3.0.0",
    "websharks/sharkicons": "160221"
  },

New Dependencies

Note that each of these require PHP 5.6+, and therefore Mobile-Adaptive Mode requires PHP 5.6+ before it will actually work. I'm going to add a note about this in the Dashboard.

    "thadafinser/user-agent-parser": "1.5.0",
    "browscap/browscap-php": "3.0.0",
@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

Referencing: Browscap Lite data source.

@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

Noting that the underlying libraries used to complete this integration will support additional data sources in the future, if we want to support them. For example, in a future release of the software we can add an additional option that allows a site owner to choose from a list of data sources and/or chain multiple data sources together.

jaswsinc
@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

2016-12-10_10-28-40

@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

2016-12-10_10-31-47

@jaswrks jaswrks added the in progress label Dec 10, 2016

@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

New Cache Sub-Directory Location

wp-content/cache/comet-cache/ua-info

Containing...

.../ua-info/browscap
@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

New Trait

includes/classes/traits/Shared/UaUtils.php
@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

2016-12-10_10-57-04

jaswsinc

@jaswrks jaswrks requested a review from raamdev Dec 10, 2016

@jaswrks

This comment has been minimized.

Copy link
Contributor

jaswrks commented Dec 10, 2016

@raamdev I'm going to leave this PR open for you to review/merge. Just let me know if you see anything wrong here, or discover something that you feel should be changed.

jaswsinc
- **New Pro Feature (Mobile-Adaptive Mode):** This release adds a new…
… feature that is designed to improve compatibility with Adaptive themes for mobile devices. To learn more, please see: **Dashboard → Comet Cache Pro → Plugin Options → Mobile-Adaptive Mode**. See also: [Issue #471](wpsharks/comet-cache#471) and the screenshots [here](#303 (comment)).
@raamdev
Copy link
Contributor

raamdev left a comment

@jaswsinc Two small change requests, otherwise this looks FANTASTIC! I ran a few tests as well and it's working awesomely. :-) 💯

echo ' </select></p>'."\n";
if (!version_compare(PHP_VERSION, '5.6', '>=')) {
echo '<p class="warning">'.sprintf(__('<strong>PROBLEM:</strong> This feature requires PHP 5.6 (or higher). You\'re currently running PHP v%1$s.', SLUG_TD), esc_html(PHP_VERSION)).'</p>'."\n";

This comment has been minimized.

@raamdev

raamdev Dec 13, 2016

Contributor

This needs the following changes:

  • Should be class="error"
  • The prefix should say "PHP Version:" instead of "PROBLEM:"
  • "PHP 5.6" needs a v: "PHP v5.6"
  • An additional note should be added to indicate where to get help. "Please contact your web hosting company for assistance."

2016-12-12_21-49-52

This comment has been minimized.

@jaswrks

jaswrks Dec 13, 2016

Contributor

Cool. Thanks for catching these. I'll update this PR shortly.

echo '<div class="plugin-menu-page-panel'.(!IS_PRO ? ' pro-preview' : '').'">'."\n";
echo ' <a href="#" class="plugin-menu-page-panel-heading" data-pro-version-only="'.(!IS_PRO ? __('pro version only', SLUG_TD) : '').'">'."\n";
echo ' <i class="si si-tablet"></i> '.__('Mobile-Adaptive Mode', SLUG_TD)."\n";

This comment has been minimized.

@raamdev

raamdev Dec 13, 2016

Contributor

"Mobile-Adaptive Mode" is a mouthful and is a bit confusing and 'unwelcoming' as a title. I suggest we use the more common name of simply "Mobile Mode" in the title and then explain "Mobile-Adaptive Mode" elsewhere inside the option panel.

In short, I suggest changing this title to "Mobile Mode" to make it easier to find and easier to identify as "the area where you configure things related to mobile devices/browsing + caching". Everything else in the panel and in the source code can stay the same.

2016-12-12_21-52-38

This comment has been minimized.

@jaswrks

jaswrks Dec 13, 2016

Contributor

Sure. Sounds good to me also.

jaswsinc

@raamdev raamdev closed this Dec 13, 2016

@raamdev raamdev deleted the feature/471 branch Dec 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment