update the highlight effect display for the confirm popup

[ShirayukiRin]

---

• I understand that contributing to this repository will require me to agree with the CLA

---

Description

Currently, the confirm popup highlights the confirm button by default, even before you hover your cursor over it. Additionally, the last button that was highlighted remains highlighted even after moving the cursor away. I think this is a bit unintuitive and could cause confusion.

video.mp4

After modification:

video.mp4

What type of PR is this? (check all applicable)

• 🍕 Feature
• 🐛 Bug Fix
• 📝 Documentation Update
• 🎨 Style
• 🧑‍💻 Code Refactor
• 🔥 Performance Improvements
• ✅ Test
• 🤖 Build
• 🔁 CI

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

Added tests?

• 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help

Added to documentation?

• 📜 README.md
• 📓 tech-docs
• 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[ra3orblade]

Can't accept this, it works with a keyboard and the logic you have added is not relevant unfortunately. We have already discussed it with designers and decided it should work like this. The code itself can be shortened this way:

either 

onMouseLeave (e: any) {
    $(this.refButtons).find('.hover').removeClass('hover');
}

or 

onMouseLeave (e: any) {
   $(e.currentTarget).removeClass('hover')
}

Changing n to null will result in an error with the next keypress as it adds arrow derection as this.n += dir;

[ShirayukiRin]

Hello,
Thank you for reviewing.
I have already added a check in the keypress event, so this should prevent any errors caused by null values.

if (this.n === null) {
	this.n = 0;
	this.setHighlight();
	return;
}

[ShirayukiRin]

According to my tests, you can still navigate using the keyboard. I haven’t removed any functionality; I’ve just set it so that no option is selected by default. You can still use the arrow keys to select an option and press Enter to confirm.

video.mp4

[ShirayukiRin]

The code itself can be shortened this way:

Thank you for your valuable suggestion! I've pushed a new commit to implement it. 😊

[ra3orblade]

if (this.n === null) {
				this.n = 0;
				this.setHighlight();
				return;
			}

this is not the best solution, you check it in arrow shortcut, while you can press enter when n is null and will still get an error.

[ShirayukiRin]

keyboard.shortcut('enter, space', e, () => {
			e.stopPropagation();
			const buttons = $(this.refButtons).find('.button');

			if (buttons[this.n]) {  // here
				$(buttons[this.n]).trigger('click');
			};
		});

I'm not entirely sure, but is the error coming from here? It seems like button[this.n] should just return undefined if this.n is null, so it doesn’t seem like it would cause an error?

[ra3orblade]

keyboard.shortcut('enter, space', e, () => {
			e.stopPropagation();
			const buttons = $(this.refButtons).find('.button');

			if (buttons[this.n]) {  // here
				$(buttons[this.n]).trigger('click');
			};
		});

I'm not entirely sure, but is the error coming from here? It seems like button[this.n] should just return undefined if this.n is null, so it doesn’t seem like it would cause an error?

It will be undefined and the enter won’t work, this is not the desired behaviour. You should be able to just press enter without hovering the button with a mouse.