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

Conditional revealed content with inline radio buttons does not render correctly #754

Closed
gavinwye opened this issue Jun 4, 2018 · 7 comments
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)

Comments

@gavinwye
Copy link

gavinwye commented Jun 4, 2018

When using the macro for a conditionally revealing element under a radio button when the buttons are displayed inline the revel element should appear under the radio buttons as shown in elements.

image

@36degrees 36degrees changed the title Conditional revel radio buttons Conditional revealed content with inline radio buttons does not render correctly Jun 4, 2018
@36degrees
Copy link
Contributor

36degrees commented Jun 4, 2018

Thanks for raising this, @gavinwye.

@adamsilver raised a good point about this pattern in Slack - when Javascript is not available the revealed thing that relates to Yes, for example, will be under No and out of context, and if there’s something to reveal for both Yes and No then similar problems arise.

This appears to be an issue with the current implementation in Elements - with Javascript enabled, the fields are associated with the radios because only one is visible at a time:

jun-04-2018 09-55-50

Whereas without Javascript, both revealed fields are visible at the same time and there is nothing to associate each field with the corresponding radio.
screen shot 2018-06-04 at 09 46 27

One suggestion is that we could somehow make it so that the 'inline' modifier is not applied when javascript is not available and there is conditionally revealing content, meaning that 'Yes' and 'No' would appear stacked with the revealed content underneath each radio button.

@timpaul
Copy link
Contributor

timpaul commented Jun 4, 2018

Thanks for spotting this @gavinwye - we'll investigate to see what we can do to improve the layout of that use case.

In the meantime we'd recommend either not using the inline layout in combination with conditionally revealed content or moving the conditional content onto another screen. For the reason @adamsilver gave, these might be preferable approaches anyway.

@NickColley NickColley added the 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) label Jun 5, 2018
@adamsilver
Copy link
Contributor

Perhaps there could be some guidance to say don't use conditionals with inline radios.

@adamsilver
Copy link
Contributor

Another little thing that we might want to consider is that if you select NO, the left border lines up with YES.

@abbott567
Copy link

We have an interesting use case. The service I'm working on is for staff. As we all know, staff hate waiting for anything to load as rubbish equipment and network latency already slow everything down. So we use conditional reveals in a few places rather than loading different views.

However, if we implemented the question stacked the whole design no longer works. The way the component is built always jams the revealed content into the dom after its selector pushing the remaining inline buttons down. (see screenshot).

So the recommendation of not using conditional reveals on inline-radios is fine when it's one field. But if it's a collection of fields it pushes the remaining radios to the bottom where they lose context.

It would be good if you could position the hidden fields in the dom manually and then pass the ID into the radio component to tell it to reveal, rather than dictating the DOM order with the component.

image

@dashouse
Copy link

Hi @gavinwye, @abbott567 and @adamsilver

We spent some time looking at this, can I point you in the direction of my comment here #970 (comment) which explains why we haven't directly addressed this issue.

@kr8n3r
Copy link

kr8n3r commented Oct 17, 2018

Hi all, with #970 our recommendation is not to use conditional revealing content with inline radios, so we're forcing them to be display:block when combined, which in turn resolves issue described here.

I'll close this issue for now.

@kr8n3r kr8n3r closed this as completed Oct 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)
Projects
None yet
Development

No branches or pull requests

8 participants