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

Multiple elements simultaneously render with Element.focused attributes #47

Closed
z5h opened this issue Oct 10, 2018 · 11 comments
Closed

Multiple elements simultaneously render with Element.focused attributes #47

z5h opened this issue Oct 10, 2018 · 11 comments
Labels
bug This is definitely a bug, meaning it is unintended behavior. It likely needs an ellie and a test. has-ellie This is a bug and has an ellie which demonstrates it.

Comments

@z5h
Copy link

z5h commented Oct 10, 2018

SSCCE: https://ellie-app.com/3zyY5PfnbG8a1

Expected behavior

Only one element can have focus at a time. So only one element should be decorated with Element.focused decorations at a time.

Actual behaviour

Clicking (focusing) a button will render that button and all following buttons as focused.

Versions

  • OS: OSX 10.13.6
  • Browser : verified in Safari 12.0 and Firefox 62.0.3, not yet tested elsewere
  • Elm Version 19
  • Elm UI Version 1.1.0
@peacememories
Copy link

It seems like elm-ui actually specifically does that:

.s:focus ~ .box-0px0px0px2px240-70-70-102-fs:not(.focus) {
    box-shadow: 0px 0px 0px 2px rgba(240,70,70,0.4);
}

This is a style generated by my app (the long class name is the name of the second button that "accidentally" gets focussed)
I have no idea what this css snippet is supposed to do though.

@tgelu
Copy link

tgelu commented Nov 30, 2018

@z5h - I noticed that if you use layoutWith and give it a focusStyle it works as expected https://ellie-app.com/437yYVP5V9ga1
Though something seems strange about this behavior

@r1sc
Copy link

r1sc commented Dec 11, 2018

I don't understand what that css-class is supposed to do either. I worked around this for now by putting each of my focused elements inside an el [] (...) - thereby defeating the sibling selector.

@stevensonmt
Copy link

stevensonmt commented Dec 12, 2018

Interestingly, if you add child elements the focus is no longer propagated whether using the layoutWith or not. In this ellie you can see that elements separated by children do not propagate focus but those not separated by children do.
https://ellie-app.com/48g8jYZh6zqa1

@rofrol
Copy link
Contributor

rofrol commented Jun 12, 2019

Workaround, thanks to @edkv:

wrap each button in an Element.el

https://ellie-app.com/5NtSNBJ4bzFa1

@fpapado
Copy link

fpapado commented Oct 3, 2019

We've run into this issue over at dillonkearns/elm-pages-starter#3 as well. We actually have this issue with links, not buttons. We use layoutWith and that does not change anything for the links.

Using workarounds with single elements is "kind of ok", but it is manual. I worry that this gives people more reason to avoid focus styles, in addition to the existing misunderstandings around them. @mdgriffith, is there an update on this bug?

@alexkorban
Copy link
Contributor

Also happens with elm-ui 1.1.5. #bug #has-ellie

@github-actions github-actions bot added bug This is definitely a bug, meaning it is unintended behavior. It likely needs an ellie and a test. has-ellie This is a bug and has an ellie which demonstrates it. labels Apr 30, 2020
@Orasund
Copy link

Orasund commented May 24, 2020

I'm currently working at a material design package for elm-ui. Both suggested workarounds do Not work for me:

Wrapping a button in an Element.el: https://ellie-app.com/8X2Q9V3BfVpa1

Adding an Element.el changes the width of the button. The styling of the button is exposed by my package API, so I can't just split it between the button and the el.

Using LayoutWith: https://ellie-app.com/8X2QtwWJnhka1

As you can see this workaround does not work

@rlefevre
Copy link
Contributor

@Orasund Your first example (https://ellie-app.com/8X2Q9V3BfVpa1) has a single element with the focused style when I test it with Firefox or Chrome. If I remove the wrapping el, the bug occurs. So the workaround seems to work, doesn't it?

Also from my tests, the bug is fixed in master, and a patch release is expected soon, so you could vendor the library locally during your package development until it is released.

@Orasund
Copy link

Orasund commented May 25, 2020

Here's my source code:

button : ButtonStyle msg -> Button msg -> Element msg
button style { onPress, text, icon } =
    Input.button
        (style.container
            ++ (if onPress == Nothing then
                    style.ifDisabled

                else
                    style.otherwise
               )
        )
        { onPress = onPress
        , label =
            Element.row style.labelRow
                [ icon |> Element.map never
                , Element.text text |> Element.el style.text
                ]
        }

As you can see the styling of the button is defined in my ButtonStyle type. We also have some styling within the label.

I need to ensure that

  1. If the button is focused, everything is focused and not only the label
  2. The size of the Button can be set in the ButtonStyle

Adding a Element.el [] outside the Input.button breaks 2. and adding it inside the label breaks 1.
So this workaround does not work for me.


It's good to know that the bug is fixed. When will the patch be released? I wanted to publish the package today, is it better to wait?

@mdgriffith
Copy link
Owner

Patch is now released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is definitely a bug, meaning it is unintended behavior. It likely needs an ellie and a test. has-ellie This is a bug and has an ellie which demonstrates it.
Projects
None yet
Development

No branches or pull requests