-
Notifications
You must be signed in to change notification settings - Fork 173
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
[Enhancement] Dice Roll Menu: Jump from “6” to “del” When Pressing Down #555
Comments
@jdlcdl May I do the PR or could you tell me how I can to colaborate? |
But... you already ARE colaborating, and you're off to a fine start. If you are not already in the "SeedSigner Devs" telegram group (or the main one), links to them are below:
But you don't necessarily have to, since you've worked it out here in this issue (and it appears you've tested the code too). You could then simply fork this repo, create a branch (for example, named "resolves_issue_555" or "dice_roll_menu_navigation" or however you like) so that a single branch contains the changes that you want to make. Once your changes are ready and you have pushed them to your github fork of seedsigner, you'll be offered a button to "create pull request" so that you can write a short blurb about your changes, including a line like "solves issue #555" which is github-site-magic that will create a link to this issue and also close this issue when your pr finally gets merged. Once submitted, all devs will be able to track your pr and test it themselves. If you need to do more changes after feedback, you'll just keep pushing commits to the same branch and we'll see them as they evolve. Perhaps, remember to bring-up your pr in the telegram group now-and-again until merged (if you think we've forgotten about it). Welcome @fedebuyito!, and thank you for your contribution thus far. |
Thank you @jdlcdl for your kindly answer and advices. I go to do you recomend me. Yes, I have tested the code with https://github.com/enteropositivo/seedsigner-emulator/tree/master tool, prior to acquire the hardware. Maybe others could to test the fork on real platform. See you on telegram group! |
Growning "del" button size property (from 2 to 3) solves navigation issue ("6th dice" -> "del"). solves issue SeedSigner#555
This is a broader question than just the "DEL" handling for this one screen. The UI / UX has thus far been consistent about the open hanging space on the final line. So I think we should be cautious about changing that behavior on just one of the keyboard screens. That being said, (more below) I don't know that there's an ideal single consistent solution. There are at least 3 possible paths here:
I dislike the idea of spanning DEL across an entire keyboard. The way it's implemented above, it looks like the screen's main Call to Action (e.g. a "Done" button; tagging in @easyuxd). At a minimum it needs to be a shorter button height to at least slightly differentiate it from our normal menu buttons. But even with that change, I just don't think the very minor UX gain is worth the worse UI presentation. Could also use 3.) on this screen instead. I wouldn't mind shrinking the dice keys a bit to create the horizontal room to put DEL on KEY2. Note: 4.) has some annoying unsolvable inconsistencies. On the dice screen, if you go DOWN from 6, you'd get a diagonal move to DEL. Okay. But let's say that was an accidental move. So you press UP to go back. Do you land back on 6? What if you do the same, but on purpose. You click DEL and now you want to navigate up to 4. But you get annoyed that UP from DEL now jumps diagonally to land you on 6. I can't recall how it was implemented when it spans 2+ keys. My guess is that it remembers your entry column and returns you there. That being said, the first two screenshots (custom derivation path, bip85 child index) could also use solution 2.) since there's room to use the middle (KEY2) button. DEL as KEY2 is definitely a better UX and faster. Only downside is that we can't use that for mnemonic word entry nor passphrase entry. So we'd have two different DEL button UXs. The 3rd screenshot (mnemonic word entry) could expand DEL to fill the gap to the right edge. That feels like an easy net win. The 4th screenshot (passphrase entry) is basically hopeless because of the third row. |
Trying to apply additional issues @kdmukai proposes, I achieved to fit del button on those screens, regarding to approach n.2 he suggets (keeping one single DEL button UXs): |
I think this is a solid enhancement. @fedebuyito: You've done a good job of keeping the Del change consistent across the input screens. Users will benefit from the larger Del key and the improved navigation that comes with it. I would love to see some before-and-after screenshots if you're able. The video you posted is helpful for interactions but a GIF is hard to navigate and pause. @kdmukai: I agree with your hesitancy around a larger Delete key looking like a CTA. Although looking at this closer, the Delete key not having a fill helps to de-emphasize it despite its increased size. No confusion on my part (not sure what others think?) but of course I defer to you because you designed these input screens and know them best. |
Thank you for your comments @easyuxd and @kdmukai. Yes, sure. Here they go (BEFORE/AFTER) screens: (Some ones keep the same, but code needed touched)
ToolsDiceEntropyEntryScreen (issue first one): ToolsCoinFlipEntryScreen (this one I missed on gif animation):
SeedBIP85SelectChildIndexScreen: SeedExportXpubCustomDerivationScreen: Would are more involucred screens(?) Regards. - |
When the dice with the number “6” is selected, pressing the down button currently leads to the go-back element “<”.
However, it might be more consistent if the behavior were the same as when the dice with the numbers “4” or “5” is selected. In those cases, pressing down takes you to the “del” element as expected.
The text was updated successfully, but these errors were encountered: