feature(UI): Help modal form

[PedroRamos360]

Checklist:

• I have read and followed the contribution guidelines.
• I have read and followed the how to open a pull request guide.
• My pull request targets the main branch of freeCodeCamp.
• I have tested these changes either locally on my machine, or GitPod.

Closes #52223

I read the suggestions in the issue #52223 and tried to create a first version of the modal. Lots of the comments have diversions but I built the form in a way that makes sense to me, I am open to suggestions and implementing changes as well as refactoring the code to best meet the guidelines.

Here are some pictures of what the form looks like:
After clicking on the help button:

After clicking on create help post on the forum:
Submit is active (user clicked in both checkboxes and typed at least 100 characters):

Submit not active

After clicking on the submit button the user is redirected to the page to create a help post with his description automatically filled in.

Building this feature I had some doubts about the free code camp code architecture that I would appreciate if anyone could help me with such as:

• How is the process to implementing new translations in the codebase?
• What components are the standard to use? The form itself I didn't know what to use, I saw some places where it was being used normal forms others imported from libraries.
• Is it okay to style I component directly using style tag? Should I create a .css file to style each component or does the application uses bootstrap or some other styling framework?

[naomi-lgbt]

I'm not sure we should lose the two links in the process.

[ilenia-magoni]

• How is the process to implementing new translations in the codebase?

The strings are added to translations.json, and accessed with the t function

Learn more: https://contribute.freecodecamp.org/#/how-to-work-on-localized-client-webapp?id=how-to-work-on-localized-client-webapp

[PedroRamos360]

I'm not sure we should lose the two links in the process.

What do you mean losing the two links? I didn't change the original modal, I just changed the functionality of the "create a help post on the forum" button to show the form instead of directly redirecting the user to the forum.

[PedroRamos360]

• How is the process to implementing new translations in the codebase?

The strings are added to translations.json, and accessed with the t function

Learn more: https://contribute.freecodecamp.org/#/how-to-work-on-localized-client-webapp?id=how-to-work-on-localized-client-webapp

Thank you! I am going to read that and try to implement the translations now.

[camperbot]

Thanks for your pull request.

Please remove the changes made to the non-English versions of the files. No need to close this pull request; just add more commits as needed.

We require you to change only English versions of files in the codebase. Translations to corresponding files in other world languages are managed on our translation platform. Once your pull request is merged, changes will be synced automatically to other world languages.

Please visit our contributing guidelines to learn more about translating freeCodeCamp's resources.

As always, we value all of your contributions.

Happy contributing!

---

Note: This message was automatically generated by a bot. If you feel this message is in error or would like help resolving it, feel free to reach us in our contributor chat.

[raisedadead]

What do you mean losing the two links?

The modal code doesn't seem to use any hyperlinks to the web pages to the "Read-Search-Ask" and "already answered...". They are just text checkboxes. People need to be able to click a link and visit the pages and then confirm they have actually read the documentation.

[PedroRamos360]

The modal code doesn't seem to use any hyperlinks to the web pages to the "Read-Search-Ask" and "already answered...". They are just text checkboxes. People need to be able to click a link and visit the pages and then confirm they have actually read the documentation.

The links were already been shown in the first part of the modal, but I added them to the checkboxes description as well now

[raisedadead]

The links were already been shown in the first part of the modal...

Thanks - I missed that. In any case it is better and somewhat expected that the links be in the checkboxes too.

This is looking good. Some one will have this reviewed soon. Thanks a lot for working on this.

[Sembauke]

Thank you for this pull-request @PedroRamos360
I am having some issues loading the link text though even after running clean:client:

Perhaps I am missing something quite obvious?

[Sembauke]

also should align the text left in help-modal.css:

a {
text-align: left;
}

[PedroRamos360]

@Sembauke Thanks for the review! I didn't know what command to run to apply the translations before that is why I didn't catch that error. Now I ran it and applied your suggested changes.

[bbsmooth]

Sorry, I added a few more accessibility updates, but I promise I'll stop now. There are still some accessibility issues that need to be addressed, but they are complicated enough that I'll save them for separate PRs.

[PedroRamos360]

@bbsmooth thank you for your suggestions! Applied them now and tested and its working as expected. I didn't really understand the necessity of removing the tab focus on a disabled button for accessibility, does it have to deal with screen readers or something like that? Also if it is the most correct for a disabled button to not be focused maybe we could implement this directly in the Button component to avoid inconsistencies.

[bbsmooth]

It's about consistency and expectations. If something can receive keyboard focus then the implication is that it is functional, and that might be confusing to some people. Also, the color contrast on disabled controls is allowed to go below minimum contrast requirements (which ours does), as long as the are truly disabled, which means no keyboard focus. So unless we want to make the text on that disabled Submit button a lot darker, then it can't allow keyboard focus.

[lasjorg]

Do we need better visual feedback for the check box requirement? When filling out the text but forgetting to check the boxes it looks like the form is just not reacting to reaching the required text amount.

Should clicking outside the second modal screen close it now that it doesn't remember what you typed? There is some potential for accidentally losing what you typed with this approach.

[PedroRamos360]

Do we need better visual feedback for the check box requirement? When filling out the text but forgetting to check the boxes it looks like the form is just not reacting to reaching the required text amount.

Should clicking outside the second modal screen close it now that it doesn't remember what you typed? There is some potential for accidentally losing what you typed with this approach.

What do you mean does not react to reaching the required text amount? If you mean that the submit button keeps disabled that is intentional as it is described above the checkboxes that it is necessary to check them so the user can continue.

[lasjorg]

I'm saying if you forget to check the boxes and start typing and get "caught up" by the "enter this many characters" when you reach that and the form still can not be submitted it isn't super obvious what is missing.

Unlike the minimum characters requirement, there is no visual indication of the checkbox requirement.

Edit: I'm only asking because I forgot to check the boxes and for a brief moment thought the logic for the text input had broken.

I'm suggesting maybe having some kind of validation error state telling the user what is missing from their form submission.

[bbsmooth]

Do we need better visual feedback for the check box requirement?

Yes, the form needs better error handling in general. This is an issue I was going to work on in a separate PR. I didn't want to drag this one out any longer.

Should clicking outside the second modal screen close it now that it doesn't remember what you typed?

That's a good point. Way too easy to accidentally click outside the box.

[huyenltnguyen]

@PedroRamos360 Thank you for the PR. And thank you everyone for the inputs and reviews.

To shorten the review loop, I've gone ahead and make some changes to the PR, as we both would like to have this feature in and to unblock some modal-related work.

Screenshot of the modal after adjustments

Changes that worth mentioning are:

• Moving inline styles to the CSS file, and using class names as selectors instead of element names
• Wrapping the input elements for the checkbox inside a label, instead of adding aria-label to the inputs
• Allowing keyboard focus on the submit button, even if the button is disabled (I'll explain in a bit)
• Sending the description from the UI component to Redux via the createQuestion action, instead of retrieving the description using document.getElementById() within Redux
• Adding tests to cover the behavior of the submit button, as well as updating tests to use the getByRole() query

---

Regarding the submit button,

When an element has the disabled attribute set, the element doesn't receive focus or react to any interactions. This is bad UX when it comes to form.

If a non-sighted user uses Tab to navigate through the form, they would only hear the inputs and the cancel button. The submit button, with disabled being true, is not announced as all, as if it is not in the DOM. This behavior could potentially confuse the user, as they don't know if there is a submit button on the page, let alone it being disabled.

I took this concern into account when I designed the shared Button component. The component accepts a disabled prop, but under the hood, it passes the value to the aria-disabled, rather than the disabled attribute.

This decision was also inspired by:

• https://css-tricks.com/making-disabled-buttons-more-inclusive/
• [New] no-disabled rule jsx-eslint/eslint-plugin-jsx-a11y#830 (the discussion is interesting to read, but if you need a TL;DR jump to [New] no-disabled rule jsx-eslint/eslint-plugin-jsx-a11y#830 (review))

If this implementation results any a11y issues, that's an oversight on my part, and I'd like to address the issue on the library level instead of spot fixing in /learn.

That was to give you the context of the implementation. But we have this discussion because we need the disabled state, which is part of what's needed in order to remove Bootstrap. Once we are have total control over our UI and our UI component library, we can revisit the current UI/UX and adjust as needed.

IMHO, the submit button shouldn't be disabled as disabling it takes away our opportunity to guide the user or to give them feedback. But instead, we could enable the button at all time, but add some validation in its click handler function to block submission if the form is invalid, and display some error messages in a live region, telling the user what they need to change.

We are not in a position to invest in these UX improvements yet, but this topic has been living rent free in my head for quite a while.

[raisedadead]

This is now live in production! Thanks @PedroRamos360 for working on this.

[bbsmooth]

@huyenltnguyen

Wrapping the input elements for the checkbox inside a label, instead of adding aria-label to the inputs

We have links in the label text for the checkboxes (which is kind of an accessibility anti-pattern to begin with). Because those links open a new window, it is considered a best practice to let screen reader users know that, which is usually done with visually hidden "opens in new window" text immediately after the link. But we don't want screen reader users to hear that visually hidden text for the check box name itself, thus we were using aria-label for the checkbox to get around that. So the trade-off here is that our code is a little cleaner but screen reader users aren't told that the link opens a new window. Not a show stopper, just wanted to make it clear why we were using aria-label. Personally, I would love to get rid of those links in the label text, especially since we have them already linked on the intro dialog page.

Concerning the tabindex issue on the disabled button, most accessibility professionals interpret WCAG to mean that that an element can only be considered disabled if it cannot be interacted with in any way, including having keyboard focus by tabbing to it. So removing tabindex="-=1" from the button does no longer makes it truly disabled and thus it must meet minimum contrast requirements to satisfy WCAG 1.4.3. (Yes, I am saying that the CSS Tricks article you linked to above is misinterpreting 1.4.3). So technically, the "disabled" Submit button in the dialog needs to have darker "Submit" text. But I wholeheartedly agree with you, we should not be disabling the Submit button in the first place and thus we won't need to worry about tabindex or contrast.

Also, I just checked production, and currently the Submit button is not disabled (even though it looks like it). I am able to click on it with both the keyboard and the mouse and get feedback from the form. The form is using the default error handling features build into the browser. They aren't perfect, but they will do for now. So I think it makes sense to get rid of aria-disabled on the Submit now.