Skip to content

Commit

Permalink
fix(StatusSeedPhraseInput): accept a common prefix suggestion
Browse files Browse the repository at this point in the history
- fix for a cornder when there was a valid seedphrase word (e.g. cat)
that is at the same type a prefix for other valid suggestions (e.g.
category, catalog, ...); it was imposible to select it using Enter key
or mouse click from the suggestions popup at the last field
- add a QML regression test for this issue
- use standard subcomponents (StatusDropdown, StatusListView) reducing
code duplication and unifying UI/UX

Fixes #16291
  • Loading branch information
caybro committed Sep 9, 2024
1 parent e87a4f9 commit 74c9ea2
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 46 deletions.
49 changes: 46 additions & 3 deletions storybook/qmlTests/tests/tst_EnterSeedPhrase.qml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,52 @@ Item {
waitForRendering(itemUnderTest)
}

// regression test for https://github.com/status-im/status-desktop/issues/16291
function test_threeLetterPrefixSuggestionInput() {
const commonPrefixToTest = "cat"

//generate a seed phrase
const expectedSeedPhrase = ["abandon", "ability", "able", "about", "above", "absent", "absorb", "abstract", "absurd", "abuse", "access", commonPrefixToTest]
const baseDictionary = [...expectedSeedPhrase, "cow", "catalog", "catch", "category", "cattle"]

let isSeedPhraseValidCalled = false
itemUnderTest.isSeedPhraseValid = (seedPhrase) => {
verify(seedPhrase === expectedSeedPhrase.join(" "), "Seed phrase is not valid")
isSeedPhraseValidCalled = true
return true
}

itemUnderTest.dictionary.append(baseDictionary.map((word) => ({seedWord: word})))

//Type the seed phrase except the last word
const str = expectedSeedPhrase.join(" ")
for (let i = 0; i < str.length - commonPrefixToTest.length; i++) {
keyPress(str.charAt(i))
}

const lastInputField = findChild(itemUnderTest, "enterSeedPhraseInputField12")
verify(!!lastInputField)
mouseClick(lastInputField)
tryCompare(lastInputField, "activeFocus", true)

// type the common prefix -> "cat..."
keyClick(Qt.Key_C)
keyClick(Qt.Key_A)
keyClick(Qt.Key_T)
tryCompare(lastInputField, "text", "cat")

// hit Enter to accept "cat"
keyClick(Qt.Key_Enter)
verify(isSeedPhraseValidCalled, "isSeedPhraseValid was not called")

// hit Enter to submit the form
keyClick(Qt.Key_Enter)
verify(itemUnderTest.submitSpy.count === 1, "submitSeedPhrase signal was not emitted")
// This signal is emitted multiple times due to the way the seed phrase is updated and validated
// The minimum is the length if the seed phrase
verify(itemUnderTest.seedPhraseUpdatedSpy.count >= expectedSeedPhrase.length, "seedPhraseUpdate signal was not emitted")
}

function test_componentCreation() {
verify(itemUnderTest !== null, "Component creation failed")
}
Expand Down Expand Up @@ -310,7 +356,6 @@ Item {
}

keyClick(Qt.Key_Enter)
print (itemUnderTest.seedPhraseUpdatedSpy.count)
verify(itemUnderTest.submitSpy.count === 0, "submitSeedPhrase signal was emitted")
verify(itemUnderTest.seedPhraseUpdatedSpy.count >= expectedSeedPhrase.length, "seedPhraseUpdate signal was not emitted")
}
Expand Down Expand Up @@ -354,10 +399,8 @@ Item {
keyClick(Qt.Key_Enter)

verify(isSeedPhraseValidCalled, "isSeedPhraseValid was not called")
print (lastVerifiedSeedPhrase)
verify(lastVerifiedSeedPhrase === expectedSeedPhrase.join(" ").slice(0, -1) + "a", "Seed phrase is not updated")
verify(itemUnderTest.seedPhraseUpdatedSpy.count >= expectedSeedPhrase.length, "seedPhraseUpdate signal was not emitted")

}

// Test suggestions are active after the seed phrase word is fixed
Expand Down
2 changes: 1 addition & 1 deletion storybook/stubs/shared/stores/BIP39_en.qml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ ListModel {

Component.onCompleted: {
var englishWords = [
"apple", "banana", "cat", "dog", "elephant", "fish", "grape", "horse", "ice cream", "jellyfish",
"apple", "banana", "cat", "cow", "catalog", "catch", "category", "cattle", "dog", "elephant", "fish", "grape", "horse", "ice cream", "jellyfish",
"kiwi", "lemon", "mango", "nut", "orange", "pear", "quail", "rabbit", "strawberry", "turtle",
"umbrella", "violet", "watermelon", "xylophone", "yogurt", "zebra"
// Add more English words here...
Expand Down
55 changes: 14 additions & 41 deletions ui/StatusQ/src/StatusQ/Controls/StatusSeedPhraseInput.qml
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,13 @@ Item {
property string leftComponentText: ""
/*!
\qmlproperty ListModel StatusSeedPhraseInput::inputList
This property holds the filtered words list based on the user's
input text.
This property holds the seed words dictionary
*/
property ListModel inputList: ListModel { }
/*!
\qmlproperty ListModel StatusSeedPhraseInput::filteredList
This signal is emitted when the user selects a word from
the suggestions list, either by clicking on it or by completing
typing 4 charactersnd passes as a parameter the selected word.
The corresponding handler is \c onDoneInsertingWord
This property holds the filtered words list based on the user's
input text.
*/
property ListModel filteredList: ListModel { }
/*!
Expand Down Expand Up @@ -107,6 +104,7 @@ Item {
let seedWordTrimmed = seedWord.trim()
seedWordInput.input.edit.text = seedWordTrimmed
seedWordInput.input.edit.cursorPosition = seedWordInput.text.length
filteredList.clear()
root.doneInsertingWord(seedWordTrimmed)
}

Expand All @@ -116,20 +114,14 @@ Item {
}
}

QtObject {
id: d

property bool isInputValidWord: false
}

Component {
id: seedInputLeftComponent
StatusBaseText {
leftPadding: 4
rightPadding: 6
text: root.leftComponentText
color: seedWordInput.input.edit.activeFocus ?
Theme.palette.primaryColor1 : Theme.palette.baseColor1
font.pixelSize: 15
}
}

Expand All @@ -140,7 +132,6 @@ Item {
input.leftComponent: seedInputLeftComponent
input.acceptReturn: true
onTextChanged: {
d.isInputValidWord = false
filteredList.clear();
let textToCheck = text.trim().toLowerCase()

Expand All @@ -149,10 +140,9 @@ Item {
}

for (var i = 0; i < inputList.count; i++) {
if (inputList.get(i).seedWord.startsWith(textToCheck)) {
filteredList.insert(filteredList.count, {"seedWord": inputList.get(i).seedWord});
if(inputList.get(i).seedWord === textToCheck)
d.isInputValidWord = true
const word = inputList.get(i).seedWord
if (word.startsWith(textToCheck)) {
filteredList.insert(filteredList.count, {"seedWord": word})
}
}

Expand Down Expand Up @@ -203,7 +193,7 @@ Item {
}
}

Popup {
StatusDropdown {
id: suggListContainer
contentWidth: seedSuggestionsList.width
contentHeight: ((seedSuggestionsList.count <= 5) ? seedSuggestionsList.count : 5) *34
Expand All @@ -215,35 +205,18 @@ Item {
rightPadding: 0

visible: ((filteredList.count > 0) && seedWordInput.input.edit.activeFocus)
background: Rectangle {
id: statusMenuBackgroundContent
color: Theme.palette.statusMenu.backgroundColor
radius: 8
layer.enabled: true
layer.effect: DropShadow {
anchors.fill: parent
source: statusMenuBackgroundContent
horizontalOffset: 0
verticalOffset: 4
radius: 12
samples: 25
spread: 0.2
color: Theme.palette.dropShadow
}
}
ListView {

StatusListView {
id: seedSuggestionsList
width: ((seedSuggestionsList.contentItem.childrenRect.width + 24) > root.width) ? root.width
: (seedSuggestionsList.contentItem.childrenRect.width + 24)
width: (((seedSuggestionsList.contentItem.childrenRect.width + 24) > root.width) ? root.width
: (seedSuggestionsList.contentItem.childrenRect.width + 24)) + 8
anchors.top: parent.top
anchors.bottom: parent.bottom

onCountChanged: {
seedSuggestionsList.currentIndex = 0
}

clip: true
ScrollBar.vertical: ScrollBar { }
model: root.filteredList

delegate: Item {
Expand All @@ -259,7 +232,7 @@ Item {
StatusBaseText {
id: suggWord
anchors.left: parent.left
anchors.leftMargin: 14
anchors.leftMargin: 8
anchors.verticalCenter: parent.verticalCenter
text: seedWord
color: mouseArea.containsMouse || index === seedSuggestionsList.currentIndex ? Theme.palette.indirectColor1 : Theme.palette.directColor1
Expand Down
1 change: 0 additions & 1 deletion ui/imports/shared/panels/EnterSeedPhrase.qml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ ColumnLayout {
let mnemonicString = ""

if (d.allEntriesValid) {

mnemonicString = buildMnemonicString()
if (!Utils.isMnemonic(mnemonicString) || !root.isSeedPhraseValid(mnemonicString)) {
root.setWrongSeedPhraseMessage(qsTr("Invalid seed phrase"))
Expand Down

0 comments on commit 74c9ea2

Please sign in to comment.