Improved confirm #65
Improved confirm #65
Conversation
chengdashun
commented
Jun 20, 2017
chengdashun
commented
|
Hi - thanks for the contribution , there are a few changes I would like to suggest. Please don't take that as discouragement and if you need some help just ask :) |
src/Dialogue/Confirm.php
Outdated
| @@ -8,6 +8,137 @@ | |||
| class Confirm extends Dialogue | |||
| { | |||
|
|
|||
| private $yesText = 'Yes'; | |||
There was a problem hiding this comment.
I think instead of changing the Confirm behaviour we could have a new dialogue type? maybe a YesNo dialogue ?
src/Dialogue/Confirm.php
Outdated
|
|
||
| $promptWidth = mb_strlen($this->text) + 4; | ||
| $fillWidth = $promptWidth - (mb_strlen($this->getYesText()) + mb_strlen($this->getNoText())); | ||
| $placeHolderWidth = 0 == ($fillWidth % 2) ? 2 : 1;//中间位宽度 |
There was a problem hiding this comment.
Could we have this comment in english please?
There was a problem hiding this comment.
OK, sorry, this is my first attempt to submit the code 😅
There was a problem hiding this comment.
No problem, take all the time you need and feel free to ask for any help! :)
| @@ -163,6 +163,10 @@ public function getKeyedInput() | |||
| "\n" => 'enter', | |||
| "\r" => 'enter', | |||
| " " => 'enter', | |||
| "\033[D" => 'left', | |||
There was a problem hiding this comment.
Are these common values for navigating?
There was a problem hiding this comment.
Sorry, I need to confirm this
There was a problem hiding this comment.
I am not very sure,because the need for left and right arrow keys
The result is that I refer to the previous key-value debugging print out
Codecov Report
@@ Coverage Diff @@
## master #65 +/- ##
============================================
- Coverage 88.76% 78.83% -9.94%
- Complexity 239 254 +15
============================================
Files 18 18
Lines 730 822 +92
============================================
Hits 648 648
- Misses 82 174 +92
Continue to review full report at Codecov.
|
|
😅 sorry PHP5.6 , 7 ,HHVM did not succeed, I am in the local test and then submit |
|
Looking good @chengdashun ! I will try it out and push some unit tests to this branch and get it merged. I will ping you when I add the unit tests so maybe you can see how to do them in the future :) |
|
@chengdashun I noticed that the options don't have padding around the The confirmation prompt looks like:
|
|
@chengdashun Any update? I would love to use that confirm dialog as well! |
|
Closing this due to lack of activity. Feel free to reopen, would be nice to have this feature! |
