gluonhq / attach

GNU General Public License v3.0
49 stars 26 forks source link

DisplayService::hasNotch always returns false #323

Closed ennerf closed 2 years ago

ennerf commented 2 years ago

Devices with notch (e.g. "iPhone12 Pro" simulator) return that they have no notch and subsequently load a wrong stylesheet with not enough margin.

I'm not familiar enough with Objective C to find the issue with the existing code, but the following Java workaround works for all devices I tested.

public static boolean hasNotch() {
    return Platform.isIOS() && displayService.map(service -> {
        // https://stackoverflow.com/a/46192822/3574093
        // https://kapeli.com/cheat_sheets/iOS_Design.docset/Contents/Resources/Documents/index
        // https://apple.fandom.com/wiki/Notch
        var dim = service.getScreenResolution();
        var height = (int) Math.max(dim.getWidth(), dim.getHeight());
        switch (height) {
            case 2778:  // iPhone 6.7-inch : iPhone 12 Pro Max
            case 2688:  // iPhone 6.5-inch : iPhone XS Max, iPhone 11 Pro Max
            case 2532:  // iPhone 6.1-inch : iPhone 12, iPhone 12 Pro
            case 1792:  // iPhone 6.1-inch : iPhone XR, iPhone 11
            case 2436:  // iPhone 5.8-inch : iPhone X,iPhone XS, iPhone 11 Pro
            case 2340:  // iPhone 5.4-inch : iPhone 12 Mini
                return true;
            case 1920: // iPhone 5.5-inch (Physical) : iPhone 6 Plus, iPhone 6S Plus, iPhone 7 Plus, iPhone 8 Plus
            case 2208: // iPhone 5.5-inch (Virtual) : iPhone 6 Plus, iPhone 6S Plus, iPhone 7 Plus, iPhone 8 Plus
            case 1334:  // iPhone 4.7-inch : iPhone 6, iPhone 6S, iPhone 7, iPhone 8, iPhone SE2
            case 1136:  // iPhone 4-inch : iPhone 5, iPhone 5S, iPhone 5C, iPhone SE
            case 960:  // iPhone 3.5-inch : iPhone 4, iPhone 4S
            case 480:  // iPhone (Legacy) & iPod Touch : 1st, 2nd and 3rd Generation
            case 2732:  // iPad Pro 12.9-inch : iPad Pro 12.9-inch 1st, 2nd, 3rd and 4th Generation
            case 2388:  // iPad Pro 11-inch : iPad Pro 11-inch 1st and 2nd Generation
            case 2224:  // iPad 10.5-inch : iPad Pro 10.5, iPad Air 3rd Generation
            case 2160:  // iPad 10.2-inch : iPad 7th Generation
            case 2048:  // iPad 9.7-inch Retina  : iPad 3rd, iPad 4th, iPad Air 1st, iPad Air 2nd, iPad Pro 9.7-inch, iPad 5th, iPad 6th Generation
                // iPad Mini Retina  : iPad Mini 2nd, 3rd, 4th and 5th Generation
            case 1024:  // iPad (Legacy): 1st and 2nd Generation && iPad Mini (Legacy): iPad Mini 1st Generation
            default:
                return false;
        }
    }).orElse(false);
}
jperedadnr commented 2 years ago

Which version of Attach are you using? 4.0.15 has some changes that should work.

ennerf commented 2 years ago

You're right, it works with 4.0.15 👍 Thanks.

ennerf commented 2 years ago

Fwiw, it looks like stylesheet loading in charm 6.1.1 still has issues. The glisten_ios_notch.css stylesheet gets loaded correctly with attach 4.0.15, but the "iPad Pro (12.9-inch) (5th generation)" simulator loads glisten_ios.css instead of glisten_ios_tablet.css.

jperedadnr commented 2 years ago

But that is unrelated to the notch? It uses DisplayService::isTablet, which in turn calls !isPhone.

ennerf commented 2 years ago

Yes, it is independent of notch, but to me related in the sense that both issues require replacing the stylesheet after initialization. DisplayService::isTablet returns the correct value, so there might be a bug with how _tablet gets added in Glisten. I didn't see a Glisten specific issue tracker though, otherwise I'd create a new issue there.

This works for me

public static String getPlatformSpecificStylesheetName() {
    String platformSuffix = PlatformUtil.isIOS() ? "_ios"
            : PlatformUtil.isAndroid() ? "_android"
            : "";
    String formFactorSuffix = PlatformUtil.hasNotch() ? "_notch"
            : PlatformUtil.isTablet() ? "_tablet"
            : "";
    return String.format("glisten%s%s.css", platformSuffix, formFactorSuffix);
}
jperedadnr commented 2 years ago

Again, not related to this current issue. Nonetheless, that is exactly what Glisten does (not apparently), but with the DisplayService for notch and tablet. I'll report this bug to the Glisten private issue tracker.