feat(frontend): sets autofocus on select field on desktop #4456

BonomoAlessandro · 2025-01-27T10:44:06Z

Motivation

If the select token modal of the swap process is opened, the search field should automatically receive focus, but only on desktop. The exact same should happen if the manage list modal is opened.

Changes

auto focus search fields on desktop

Tests

before:

after:

BonomoAlessandro · 2025-01-27T13:38:05Z

@peterpeterparker What do you think about the device.utils.ts class? Do you see a better or cleaner solution than my implementation?

peterpeterparker

Can you provide the utility in a separate PR?
No tests?

src/frontend/src/lib/components/ui/InputTextWithAction.svelte

peterpeterparker · 2025-01-27T18:00:53Z

src/frontend/src/lib/components/ui/InputTextWithAction.svelte

+
+	onMount(() => {
+		if (autofocus) {
+			let component = document.querySelector<HTMLInputElement>(`input[name="${name}"]`);


Is there a particular reason why you're using vanilla JavaScript instead of Svelte here?

If not, could you switch to Svelte? I generally prefer to avoid mixing both approaches, as I'm not sure that it cannot lead to unexpected behavior.

Additionally, this query selector seems optimistic, as it selects any matching element from the root. There seems to be nothing preventing another component from reusing the exact same selector in the future, which could lead to unexpected behavior or malfunctions.

@peterpeterparker yes there is. If i use the svelte bind, i will get the svelete component Input from gix-components, which does not provide an option to set focus. To be able to set the focus, i use the JavaScript to directly access the input element, allowing me to set the focus.

src/frontend/src/lib/components/ui/InputTextWithAction.svelte

src/frontend/src/lib/utils/device.utils.ts

peterpeterparker · 2025-01-27T18:03:13Z

src/frontend/src/lib/utils/device.utils.ts

@@ -0,0 +1,16 @@
+import { nonNullish } from '@dfinity/utils';
+
+interface UserAgentData {


Is there a typedef (@types/...) available for this? If not, should we declare the type globally in a .d.ts file in case it ever become available in NodeJS?

Like declaring it for the navigator this way you do not have to explicitely declare the types when you use it.

Strictly a question, not a call to action.

src/frontend/src/lib/utils/device.utils.ts

src/frontend/src/lib/components/ui/InputTextWithAction.svelte

peterpeterparker · 2025-01-29T08:14:42Z

Thanks for the clarification. So, why not enhancing the Gix components directly instead of working around in OISY?

This feature could be integrated into the design kit, allowing the community and other foundation dapps, like NNS dapp, to benefit from it as well.

Alternatively, why not just exposing the HTML input element in the Gix component?

The former is probably cleaner and more meaningful but, the later would at least avoid adding a workaround which, without tests (you are going to provide some?), might fail some day.

BonomoAlessandro · 2025-01-29T08:46:09Z

@peterpeterparker I also think the cleanest solution would be to enhance the Gix components, but this will take some time. In the meantime, we want to implement a workaround on the OISY side.

As you requested in your first comment, i have provided some test cases and created a separate PR for the device.utils. You can find it here: #4493

peterpeterparker · 2025-01-29T08:51:03Z

I reviewed and approved #4493, thanks! It was a draft, but since you mentioned it, I guess it was ready.

Regarding Gix components, I don't think it takes that much more time, and I believe this workaround, as it is in this PR, should not be implemented this way. Hence, I’m personally not going to approve this PR in this form.

…ofocus-search-fields

BonomoAlessandro · 2025-02-03T09:03:20Z

@peterpeterparker I am now using the newest version of the gix-components.

peterpeterparker · 2025-02-03T09:11:37Z

package.json

@@ -57,7 +57,7 @@
 		"@dfinity/candid": "^2.1.2",
 		"@dfinity/ckbtc": "^3.1.6",
 		"@dfinity/cketh": "^3.4.3",
-		"@dfinity/gix-components": "^5.0.0-next-2024-12-10",
+		"@dfinity/gix-components": "^5.0.0-next-2025-01-31",


Does this next of Gix components version contains solely the changes for this PR or does it contains other changes? If the later, it's cleaner I think to provide the upgrade in a dedicated PR. If the former, nvm this comment.

@peterpeterparker This provides all the changes made since december 10th. I will create a separate PR for this

@peterpeterparker I just opened the PR: #4540

peterpeterparker

LGTM, thx

peterpeterparker · 2025-02-03T10:46:54Z

src/frontend/src/lib/components/ui/InputTextWithAction.svelte

+
+	onMount(() => {
+		if (autofocus && nonNullish(inputElement)) {
+			inputElement.focus();


Just a note—I assume you tested this after the change in Gix components.

Spontaneously, I would rather use an auto-subscriber, as I'm not sure the reference to the DOM element would always be established when the component is mounted. But I'm probably just being paranoid. ;)

BonomoAlessandro and others added 11 commits January 27, 2025 11:42

adds autofocus to InputTextWithAction component

100b7a9

fix lint issues

7b7bd36

🤖 Apply formatting changes

2c4b9fc

fix lint issues

b673abe

adds device utils

dc3fd9b

🤖 Apply formatting changes

8f5b407

adds custom type

57abaf1

fixes lint issues

64d6132

fixes lint issues

0d9a3db

fixes lint issues

2dd2731

Merge branch 'main' into feature(frontend)/autofocus-search-fields

26c5272

BonomoAlessandro marked this pull request as ready for review January 27, 2025 13:26

BonomoAlessandro requested a review from a team as a code owner January 27, 2025 13:26

BonomoAlessandro changed the title ~~feat(frontend): adds autofocus to InputTextWithAction component~~ feat(frontend): sets autofocus on select field on desktop Jan 27, 2025

peterpeterparker reviewed Jan 27, 2025

View reviewed changes

BonomoAlessandro added 2 commits January 29, 2025 08:15

updates device.utils.ts

3f3f438

adds return value

a4e11d0

BonomoAlessandro and others added 3 commits January 29, 2025 09:32

implements test cases for the device utils

a0d9c87

removes unused code

329f42a

🤖 Apply formatting changes

80c2049

BonomoAlessandro and others added 6 commits January 29, 2025 10:01

Merge remote-tracking branch 'origin/main' into feature(frontend)/aut…

8628f84

…ofocus-search-fields

adds auto select code

7e359ff

updates gix components dependency

1d645dd

🤖 Apply formatting changes

38837be

resolves lint issue

4c1771d

Merge branch 'main' into feature(frontend)/autofocus-search-fields

1a3d7b2

peterpeterparker reviewed Feb 3, 2025

View reviewed changes

Merge branch 'main' into feature(frontend)/autofocus-search-fields

adf85c8

peterpeterparker approved these changes Feb 3, 2025

View reviewed changes

BonomoAlessandro merged commit 0a0d9b5 into main Feb 3, 2025
21 checks passed

BonomoAlessandro deleted the feature(frontend)/autofocus-search-fields branch February 3, 2025 11:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend): sets autofocus on select field on desktop #4456

feat(frontend): sets autofocus on select field on desktop #4456

BonomoAlessandro commented Jan 27, 2025 •

edited

Loading

BonomoAlessandro commented Jan 27, 2025

peterpeterparker left a comment

peterpeterparker Jan 27, 2025

BonomoAlessandro Jan 29, 2025

peterpeterparker Jan 27, 2025 •

edited

Loading

peterpeterparker commented Jan 29, 2025 •

edited

Loading

BonomoAlessandro commented Jan 29, 2025

peterpeterparker commented Jan 29, 2025 •

edited

Loading

BonomoAlessandro commented Feb 3, 2025

peterpeterparker Feb 3, 2025

BonomoAlessandro Feb 3, 2025

BonomoAlessandro Feb 3, 2025

peterpeterparker Feb 3, 2025

peterpeterparker left a comment

peterpeterparker Feb 3, 2025

		@@ -0,0 +1,16 @@
		import { nonNullish } from '@dfinity/utils';

		interface UserAgentData {

feat(frontend): sets autofocus on select field on desktop #4456

feat(frontend): sets autofocus on select field on desktop #4456

Conversation

BonomoAlessandro commented Jan 27, 2025 • edited Loading

Motivation

Changes

Tests

BonomoAlessandro commented Jan 27, 2025

peterpeterparker left a comment

Choose a reason for hiding this comment

peterpeterparker Jan 27, 2025

Choose a reason for hiding this comment

BonomoAlessandro Jan 29, 2025

Choose a reason for hiding this comment

peterpeterparker Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

peterpeterparker commented Jan 29, 2025 • edited Loading

BonomoAlessandro commented Jan 29, 2025

peterpeterparker commented Jan 29, 2025 • edited Loading

BonomoAlessandro commented Feb 3, 2025

peterpeterparker Feb 3, 2025

Choose a reason for hiding this comment

BonomoAlessandro Feb 3, 2025

Choose a reason for hiding this comment

BonomoAlessandro Feb 3, 2025

Choose a reason for hiding this comment

peterpeterparker Feb 3, 2025

Choose a reason for hiding this comment

peterpeterparker left a comment

Choose a reason for hiding this comment

peterpeterparker Feb 3, 2025

Choose a reason for hiding this comment

BonomoAlessandro commented Jan 27, 2025 •

edited

Loading

peterpeterparker Jan 27, 2025 •

edited

Loading

peterpeterparker commented Jan 29, 2025 •

edited

Loading

peterpeterparker commented Jan 29, 2025 •

edited

Loading