PeterStaev / NativeScript-Drop-Down

A NativeScript DropDown widget.
Apache License 2.0
105 stars 65 forks source link

Update to nativescript 7 #257

Closed ReazerDev closed 3 years ago

ReazerDev commented 3 years ago

Closes https://github.com/PeterStaev/NativeScript-Drop-Down/issues/254

PeterStaev commented 3 years ago

@ReazerDev the latest changes look good to me! If you don't intend to make any other changes I can merge this and push a version on NPM?

ReazerDev commented 3 years ago

@ReazerDev the latest changes look good to me! If you don't intend to make any other changes I can merge this and push a version on NPM?

@PeterStaev Is everything working on iOs? I can't test there since I don't have any apple devices xD If it does, then I don't intend to make any changes👍

PeterStaev commented 3 years ago

@ReazerDev , if no one else can test, I will try to test it later this week or the next one. Then will see to merge and push to NPM.

Archez commented 3 years ago

I did some testing for iOS. The plugin works fine.

The only thing is with iOS14 & XCode12, Apple changed the styling of the UIPickerView. Now the picker has a sort of margin around the selected item. This margin however is not being applied to the items inside the picker, so about 8~ pixels of the label is being cut off (maybe this is a bug in Apple's design? Here is a stackoverflow post where developers have run into this).

image

To offset this, we can check when the device version is >= 14 and apply an additional left and right padding to the labels that are created for the items. 8 pixels is enough to prevent the clipping, but 16 pixels would position the label in a way that matches Apple's official use of the picker.

Here is what it looks like with the extra 16 pixels of padding, (8 pixels would place the start of the label at the start of the lighter shaded area) image

On iOS devices < 14, the picker does not clip the labels at all image


We can add a constant at the top that checks the device version to decide any extra padding to use

const NEW_PICKER_STYLE_ROW_INSET = parseFloat(Device.osVersion) >= 14.0 ? 16 : 0;

And for DropDownListPickerDelegateImpl under the pickerViewViewForRowForComponentReusingView method, we can apply this extra padding to the label elements that are generated

// On iOS >= 14 the UIPickerView was restyled and now has a smaller area for the picker wheel
// A bug requires that we manually add padding to reposition the labels within this smaller area,
// Otherwise the labels will be clipped
label.padding = {
    top: Length.toDevicePixels(itemsPaddingTop, 0),
    right: NEW_PICKER_STYLE_ROW_INSET + Length.toDevicePixels(itemsPaddingRight, 0),
    bottom: Length.toDevicePixels(itemsPaddingBottom, 0),
    left: NEW_PICKER_STYLE_ROW_INSET + Length.toDevicePixels(itemsPaddingLeft, 0)
};

I tested the 16 pixels padding works for the different UI scaled devices.

ReazerDev commented 3 years ago

@Archez Thanks for testing! I pushed your suggested changes, would you be so kind to test it again?

ReazerDev commented 3 years ago

@PeterStaev Don't merge yet, please. I've found a problem with @nativescript/theme's Dark Mode

ReazerDev commented 3 years ago

@PeterStaev Ok done, can you check my latest commit, I'm not sure if it's a bad practice, or not, to use @nativescript/theme in a plugin

PeterStaev commented 3 years ago

@ReazerDev , I do not like to have dependecy on the theme package since there might be applications that are not using the theme at all. Aren't these changes possible to do in the app itself? Like define a class for the DD that will get applied when darkmode is enabled and another class when darkmode is disabled? I'm not an export in NS theme, but isn't something like this possible but for the DropDown:

Button {
    color: light(primary);

    .ns-dark & {
        color: dark(primary);
    }
}
ReazerDev commented 3 years ago

TbH I don't know either^^ I did some testing tho and if an app doesn't use the theme package, it works fine. Just defaults to a white background with black text.

PeterStaev commented 3 years ago

@ReazerDev As I read the code the @nativescript/theme package is added as dev dependency. So if an app does not have it installed it will probably cause errors since the import will fail. I would say let's rollback your last commit and leave to the end user to style the drop down via CSS in the app as they like.

ReazerDev commented 3 years ago

@PeterStaev Ok, removed the commit.