Skip to content
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

Binding focus state to elements #8949

Closed
Maximinodotpy opened this issue Jul 10, 2023 · 21 comments
Closed

Binding focus state to elements #8949

Maximinodotpy opened this issue Jul 10, 2023 · 21 comments
Labels
feature request good first issue A small, encapsulated issue, ideal for a new contributor to Svelte to tackle.
Milestone

Comments

@Maximinodotpy
Copy link

Describe the problem

I hope i'm not missing something but it would be cool if we were able to bin variables to focusable elements abd find out this way if there focused or not, furthermore if we set this bound variable to true it should focus this element and blur in case its turned to false.

Describe the proposed solution

A property like this bind:focused

Alternatives considered

Using the focus and blur functions for focusable elements and the activeElement property of the document.

Importance

nice to have

@brunnerh
Copy link
Member

brunnerh commented Jul 10, 2023

It would be convenient, but you already can get pretty close using actions.

Something along the lines of:

export function focused(node, store) {
    const onChange = () => store.set(document.activeElement == node);
    node.addEventListener('focus', onChange);
    node.addEventListener('blur', onChange);

    const cancel = store.subscribe(shouldFocus => {
        const focused = document.activeElement == node;
        if (focused && shouldFocus == false)
            node.blur();
        else if (focused == false && shouldFocus)
            node.focus();
    });

    return {
        destroy() {
            node.removeEventListener('focus', onChange);
            node.removeEventListener('blur', onChange);
            cancel();
        }
    }
}

Usage:

<script>
  import { writable } from 'svelte/store';
  import { focused } from './actions.js';

  const inputFocused = writable(false);
</script>

<input use:focused={inputFocused} />

REPL

Of course that is more verbose as it requires imports and stores.


For convenience the action and store could be combined into one object, e.g.

import { writable } from 'svelte/store';

export function createFocused(initialValue) {
  const store = writable(initialValue);

  function action(node) {
    // [action as above]
  }

  Object.assign(action, store);
  return action;
}
<script>
  import { createFocused } from './actions.js';
  const focused = createFocused(false);
</script>

<input use:focused />
Focused: {$focused}

REPL

@Maximinodotpy
Copy link
Author

Ah youre right it could also be done this way but would it also be possible without a store?

@brunnerh
Copy link
Member

I do not think so. You need some way to bridge reactivity between component and non-component code.

@Maximinodotpy
Copy link
Author

I'm not sure but I imagine that normal let variables internally somewhat behave like stores and other dom things like the width are also possible so I think it would be easiest to make this a built in feature, of course dont take me too seriously because I have no idea of the source code.

@StagnantIce
Copy link

StagnantIce commented Jul 29, 2023

Without store you can use callback in this case


export function focused(node, onFocused) {
       const onChange = () => onFocused(document.activeElement == node);
       node.addEventListener('focus', onChange);
       node.addEventListener('blur', onChange);
       ...
}

function onFocused(isFocused: boolean) {

}

<input use:focused={{onFocused}} />

@Rich-Harris Rich-Harris added this to the 5.x milestone Apr 3, 2024
@Rich-Harris Rich-Harris added feature request good first issue A small, encapsulated issue, ideal for a new contributor to Svelte to tackle. labels Apr 3, 2024
@RaiVaibhav
Copy link
Contributor

Hey @Maximinodotpy there is a onFocus and onBlur exists, if that still doesn't full fill your need, @Rich-Harris I would like to work on this part of the feature.

@trueadm
Copy link
Contributor

trueadm commented Apr 18, 2024

Why do we need this? You can just check if something is focused by using document.activeElement.

@RaiVaibhav
Copy link
Contributor

Kindly close the PR if it not needed

@brunnerh
Copy link
Member

I'd say the main reason to have this would be the write access:
You can focus the element declaratively without using bind:this first or using an action.

@trueadm
Copy link
Contributor

trueadm commented Apr 18, 2024

Write access is tricky with focus. If you try to bind focus to two elements by accident, you cause so many issues with accessibility. It's much better to handle focus management with a global state machine instead of on a component-by-component basis. It gets even more complex too when working with elements that can have focus but not be in scroll view – so managing scroll + focus kind of flow together.

@RaiVaibhav
Copy link
Contributor

Few things I came accross while writing a PR, if a single variable bind to two different element, binding multiple Focus on multiple elements, multiple issues related to accessibility like screen reader, scrolling behaviour.

PR cover above scenarios but again focus itself have option like preventScroll, so I am not sure, letting he user bind the focus is a good idea, because if they need something after component mount, they can use, autofocus?

@trueadm
Copy link
Contributor

trueadm commented Apr 18, 2024

Sorry, I don't think this is something we want to invest our time into now. Like I said, focus is complicated and easy to mess up and hard to debug as to why.

@trueadm trueadm closed this as completed Apr 18, 2024
@trueadm trueadm reopened this Apr 18, 2024
@trueadm
Copy link
Contributor

trueadm commented Apr 18, 2024

Actually, I think this would be best if it was read-only, rather than writable. Then I can see a way forward for the PR too :)

@RaiVaibhav
Copy link
Contributor

Let me know if I can work on this @trueadm, I would be happy to open a new PR

@trueadm
Copy link
Contributor

trueadm commented Apr 18, 2024

@RaiVaibhav Sure, go for it :)

@ecstrema
Copy link
Contributor

Has this since been removed? I can't make it work anymore...

@brunnerh
Copy link
Member

@ecstrema Seems to work in the playground.

@ecstrema
Copy link
Contributor

You are right, I was trying it on a textarea element.

Arguably it should work on any element though, shouldn't it?

@brunnerh
Copy link
Member

@ecstrema
Copy link
Contributor

It seems that my IDE is not happy, but that it is indeed working.

Image

So it looks like a typings issue. should it maybe be added to the declaration files?

@brunnerh
Copy link
Member

@ecstrema Maybe open a new issue regarding this, closed ones do not get much attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request good first issue A small, encapsulated issue, ideal for a new contributor to Svelte to tackle.
Projects
None yet
Development

No branches or pull requests

8 participants