Tutorial is wrapped in a <form> element, but is not a form

[eatyourgreens]

Expected behavior

Tutorials should be marked up as regular text.

Current behavior

Please include any error messages from the browser console and/or screenshots Tutorials are actually wrapped in <form> tags. I think this might be a bug in the underlying modal-form library, so it may apply to other dialog boxes too.

Steps to replicate

Inspect a Tutorial with browser dev tools.

[srallen]

They're forms by design. It's written to hijack the native browser form submit behavior to control the display of the dialog: https://github.com/zooniverse/modal-form/blob/master/dialog.js#L23-L47

There is a prop to specify a different tag, but I'm not sure if that would have an effect on the open/close state being managed by hijacking the form submission behavior.

What I'd like to do is rewrite this and the mini-course component in the ZRC repo to be proper modal dialogs with callback props to handle the minor differences between and a prop to specify the display size (mini-course vs standard tutorial).

[eatyourgreens]

Ok, I'll leave this open as an accessibility bug since screenreaders like JAWS will probably try to read the tutorial contents as a form.

Sounds like there's some terrible code in that modal library: 😢

[srallen]

It's a good example of when to not be clever in trying to solve a problem. Pretty much every other React modal library out there expects you to manage the open/close state in whatever app state you're using rather than being self contained like this is. I'd be happy to redo it with Grommet's Layer component.

[eatyourgreens]

That would be excellent. Are you thinking of moving dialogs over to use ZRC instead of modal-form?

[srallen]

Yep. @wgranger already added a near identical copy of the PFE tutorial using modal-form to ZRC already, so I'd really like to have a tutorial component there for use by PFE and custom projects. Since there are two copies in parallel, it'd be nice to get a stable refactored version of it done sooner than later and get rid of the duplication. Rewriting the tutorial the way I'm imagining would be a full version release for ZRC.

[eatyourgreens]

Great! I've also flagged up #1953 as zooniverse/Zooniverse-React-Components#51. The solution for that might be to just rewrite the Tutorial component anyway.

[srallen]

Just to clarify, I don't think we should write our own basic dialog. Grommet has a Layer, which in version 2 does modals and pull outs. Grommet version one has a tool tip which as of now, I'm expecting to still be available for version 2, but they haven't added it yet. I think that would cover the different modal styles that modal-form does?

[wgranger]

I was unaware Grommet had a layer component. This seems like a pretty easy modal to work with, so I might try implementing it in some work for Scribes of the Cairo Geniza.

[eatyourgreens]

I agree. Grommet should be fine. As far as I can tell, the issues in #1953 are caused because the Tutorial component renders all the slides in the DOM, then hides some with CSS positioning: https://github.com/zooniverse/Zooniverse-React-Components/blob/89a4b908fb732e511fc77f75825df2c6288f245b/src/components/tutorial.jsx#L62-L89

I think those issues would still be there regardless of how the parent dialog was implemented, so they'll need a rewrite of the Tutorial render method.

[srallen]

The tutorial slide content is an array from the API, so I think to solve that would be instead of rendering them all and hiding the rest using CSS to just track which slide the user is on in the local component state using the array index. One disadvantage to this is that the images won't be preloaded, so there will be a load time for each slide. I think images could be preloaded using javascript only without rendering to the browser though.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

Closed by probot-stale due to a lack of recent activity. Please feel free to re-open if you wish to take on this change.