FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: implement capitalization type option for prompt dialogs by hettiger · Pull Request #6325 · NativeScript/NativeScript · GitHub

/ NativeScript Public

feat: implement capitalization type option for prompt dialogs - #6325

Merged
vakrilov merged 4 commits into
NativeScript:masterfrom
hettiger:patch-3
Oct 5, 2018
ConversationCommits (4)ChecksFiles changed
Merged

feat: implement capitalization type option for prompt dialogs#6325
vakrilov merged 4 commits into
NativeScript:masterfrom
hettiger:patch-3

Conversation

hettiger commented Sep 30, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Allow setting auto capitalization to "none", "all", "sentences" and "words".
This feature works on both Android and iOS.
Closes: #6326

PR Checklist

What is the current behavior?

Prompt dialogs do not support auto capitalization.
This leads to a bad user experience and bad input.

What is the new behavior?

It's now possible to set a capitalization type on the PromptOptions object.

Example:

const options: PromptOptions = {
    title: "Title",
    message: "Message",
    inputType: inputType.text,
    okButtonText: "OK",
    cancelButtonText: "Cancel",
    cancelable: true,
    capitalizationType: capitalizationType.sentences
};

(The new property is: capitalizationType: capitalizationType.sentences)

There are no breaking changes to be expected.
The new property is optional.

Test Results

iOS

=== ALL TESTS COMPLETE ===
1612 OK, 6 failed
DURATION: 28714.39 ms
=== END OF TESTS ===
CONSOLE ERROR file:///app/tns_modules/tns-core-modules/trace/trace.js:166:30: Test: FLEXBOXLAYOUT.testJustifyContent_flexEnd_withParentPadding FAILED:  Actual: <654>(number). Expected: <686>(number)
CONSOLE ERROR file:///app/tns_modules/tns-core-modules/trace/trace.js:166:30: Test: SAFEAREALAYOUT.test_grid_component_cells_layout_in_safe_area_action_bar_hidden FAILED: cell height should be equal - cell11<432> - cell21<431> Actual: <432>(number). Expected: <431>(number)
CONSOLE ERROR file:///app/tns_modules/tns-core-modules/trace/trace.js:166:30: Test: SAFEAREALAYOUT.test_grid_component_cells_layout_in_safe_area_tab_bar FAILED: cell height should be equal - cell11<398> - cell21<399> Actual: <398>(number). Expected: <399>(number)
CONSOLE ERROR file:///app/tns_modules/tns-core-modules/trace/trace.js:166:30: Test: SAFEAREALAYOUT.test_grid_nested_grid_cells_layout_beyond_safe_area_action_bar_hidden FAILED: cell11 height<432> not less or equal cell21 height<431>
CONSOLE ERROR file:///app/tns_modules/tns-core-modules/trace/trace.js:166:30: Test: SAFEAREA-LISTVIEW.test_grid_nested_list_views_layout_in_safe_area_action_bar_hidden FAILED: cell11 height<432> not less or equal cell21 height<431>
CONSOLE ERROR file:///app/tns_modules/tns-core-modules/trace/trace.js:166:30: Test: SAFEAREA-SCROLL-VIEW.test_grid_component_cells_layout_in_safe_area_action_bar_hidden FAILED: cell11 height<432> not less or equal cell21 height<431>
CONSOLE INFO file:///app/tns_modules/tns-core-modules/trace/trace.js:160:29: Test: Tests EOF!

Android

JS: === ALL TESTS COMPLETE ===
JS: 1489 OK, 2 failed
JS: DURATION: 47099.06 ms
JS: === END OF TESTS ===
JS: Test: FLEXBOXLAYOUT.testAlignContent_spaceAround FAILED: Label<text3>.bottom:1368 is not 52.5 of FlexboxLayout<flexbox>.bottom:1422
JS: Test: FLEXBOXLAYOUT.testAlignContent_spaceAround_flexDirection_column FAILED: Label<text3>.right:906 is not 52.5 of FlexboxLayout<flexbox>.right:960
JS: Test: Tests EOF!

feat: implement capitalization type option for prompt dialogs
7ed49ae 
Allow setting auto capitalization to "none", "all", "sentences" and "words".
This feature works on both Android and iOS.
ghost added the ♥ community PR label Sep 30, 2018
hettiger mentioned this pull request Sep 30, 2018
Closed

hettiger commented Oct 1, 2018

Copy link
Copy Markdown
Contributor Author

I'm not familiar with NativeScript e2e testing. Am I expected to provide tests or does the label »e2e test needed« mean that someone of the NativeScript team will take care of it?

I'd be happy to provide the tests myself but I'll probably need some help / reviews and input to get me started on NativeScript e2e testing.

Am I on the right track? Is there anything special about this repository that differs from the guide?
Are there any additional docs to get me started that I've missed?

Please let me know how we can continue with this PR @vakrilov

vakrilov commented Oct 1, 2018

Copy link
Copy Markdown
Contributor

Hey @hettiger
First of all - thanks for the awesome PR!
The few failing test are not caused by your code, so don't worry about them.

You are not expected to provide and e2e test about this PR (the e2e test needed label is for internal use for now). It would be great if you could extend the existing test page for dialogs with buttons showing a dialogs with the new capitalization options. Our QAs will just add the e2e tests using this page once the PR is merged. Once that is ready - I think this one is ready to land in master.

hettiger commented Oct 3, 2018

Copy link
Copy Markdown
Contributor Author

Thank you for your feedback @vakrilov

  • I've added the requested buttons to the existing test page.
  • The test results remain the same after the new commit.

Let me know if there's anything missing.

vakrilov commented Oct 3, 2018

Copy link
Copy Markdown
Contributor

LGTM!
We are waiting merge some PRs that should fix the broken tests.
We will update you branch after that and hopefully everything should be green.
Nothing to do from your side for now.

vakrilov commented Oct 5, 2018

Copy link
Copy Markdown
Contributor

test

ghost assigned vakrilov Oct 5, 2018
ghost added in progress and removed ♥ community PR labels Oct 5, 2018

vakrilov commented Oct 5, 2018

Copy link
Copy Markdown
Contributor

test

vakrilov merged commit ae6a661 into NativeScript:master Oct 5, 2018
ghost removed the in progress label Oct 5, 2018
MartoYankov added the docs needed Additional documentation on this issue/PR is needed label Oct 6, 2018

radeva commented Nov 19, 2018

Copy link
Copy Markdown

Hi @hettiger ,

Congratulations on being one of the winners in the {N} First-time contributors contest!

You can claim your prize by contacting us at nativescriptwinners[at]progress.com not later than Nov 30th 2018 . Make sure you send us the following info: 
👉Your full name 
👉Your email 
👉Your country or residence 

Best regards,
The NativeScript Team

NickIliev removed the docs needed Additional documentation on this issue/PR is needed label Jan 11, 2019
This was referenced Jan 11, 2019
Merged
Merged

lock Bot commented Mar 10, 2020

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

lock Bot locked and limited conversation to collaborators Mar 10, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
vakrilov vakrilov approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

vakrilov

Labels

cla: yes ♥ community PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Prompt dialog: An option for auto capitalization is missing

7 participants

Back | FazBrowse Home | New Git URL