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

Add the ability to redirect based on a radio button chosen. #374

Closed
wants to merge 2 commits into from

Conversation

tsmorgan
Copy link
Contributor

Based on #227 with an added example from @joelanman's #338 (also covers #362 too, I think).

The main difference with this PR from previous ones is that now we have data being stored in the session it's possible to address the comments from #338 and include the option to save data at the same time as redirecting.

The example page now contains two examples. The first shows the method of redirecting via radio button values and the second shows a more complex example involving aliens.

@edwardhorsford
Copy link
Contributor

Thanks for continuing to work on this @tsmorgan.

I feel wary of using value for this. It feels somewhat unorthodox and might be hard to explain to people. What do others think?

Spitballing - what about putting a data-redirect="url" on each input, and using JS to append a hidden field that gets submitted to the server with the redirect url? That way values stay 'normal', and we're reusing data-thing to do interactive stuff - which people will be familiar with if they're doing progressive disclosure.

@tsmorgan
Copy link
Contributor Author

tsmorgan commented Apr 14, 2017

Not sure I agree. The value is where you'd put the data that you'd do more complex branching on, so it makes sense to me that you use the same thing to trigger this redirect.

@joelanman
Copy link
Contributor

joelanman commented Apr 15, 2017

I think any approach is going to be non standard HTML, but I agree with Tom's reasoning. If we went with data-redirect we'd have two attributes to teach people. I think I favour this:

<input type="radio" name="work-status" value="self-employed">

Then talk about autodata, and how to do branching in routes. Then introduce that we have a shortcut for radios:

<input type="radio" name="work-status" value="self-employed, redirect(/self-employed)">

I think it's good practice to say there's always a value, and you can optionally add our redirect function.

As I'm writing this, is it odd that the redirect isn't quoted? It doesn't need to be I don't think.

@tsmorgan
Copy link
Contributor Author

Yeah, I don't think it should be quoted. It's within with quotes of the value attribute anyway and it's within brackets. Having more quotes could confuse things and we might get into the realms of having to tell people to pick single or double quotes depending on what they'd used in their HTML.

Just realise I've got the data being passed after the redirect (eg. value="redirect(<url>), <data>") which wasn't what you guys arrived at in your discussion on #338. Also thinking about "it's good practice to say there's always a value", it does make more sense to think of this as an add-on to the normal passing of data. So if you'll bare with me I'll update the regex to expect <data> first and redirect(<url) after a comma (eg. value="<data>, redirect(<url>)").

@tsmorgan
Copy link
Contributor Author

OK that's done now. Would appreciate you guys checking it through a bit as the regex is new. It's definitely better this way around though, the explanation on the example page feels much simpler now.

@joelanman
Copy link
Contributor

Thanks @tsmorgan, so sorry for the long delay on this. We know it's a need teams have, so we've raised an issue for it: #576

Unfortunately this particular approach is now out of date (our fault), so I'm closing this pull request.

@joelanman joelanman closed this Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants