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 new Contact Us information page #3719

Merged

Conversation

chrisgarrity
Copy link
Contributor

@chrisgarrity chrisgarrity commented Feb 28, 2020

Resolves:

Moves the contact-us form from the legacy scratchr2 system to www.

Changes:

Slight update to the wording to make it a bit more friendly. Also uses the Freshdesk formwidget to submit directly to Freshdesk instead of sending an email (will allow us to remove one email path from scratchr2)

  • adds new component HelpForm for the Freshdesk component
  • adds new contact-us view
  • modifies Information page css with style for a header on the right hand nav section
  • adds new route for the contact-us page (is identical to the old scratchr2 URL)

Note: Information Page template is NOT responsive, and that is out of scope for this change.

Test Coverage:

Screen Shot 2020-02-28 at 9 23 01 AM

Testing

  • Text displays (no message ids - check other languages, but don't expect translation)
  • Form displays (only be English for now)
  • Can submit form
  • if logged in the additional info (scroll the form to view) includes Scratch username and browser info
  • if not logged in, additional info has blank username, browser is still filled in
  • browser info is correct (browser, operating system etc. should match window.navigator.userAgent in the console)
  • links from reporting profile or studio modals link to new form with relevant info in subject and description.
  • Verify that form is accessible and can be submitted from China

Copy link
Contributor

@apple502j apple502j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some questions and typos

src/components/helpform/helpform.jsx Outdated Show resolved Hide resolved
src/views/contact-us/l10n.json Outdated Show resolved Hide resolved
src/views/contact-us/l10n.json Outdated Show resolved Hide resolved
src/views/contact-us/l10n.json Show resolved Hide resolved
src/views/contact-us/l10n.json Outdated Show resolved Hide resolved
src/views/contact-us/l10n.json Outdated Show resolved Hide resolved
const FormattedMessage = require('react-intl').FormattedMessage;

const HelpForm = props => {
const prefix = 'https://mitscratch.freshdesk.com/widgets/feedback_widget/new?&widgetType=embedded&widgetView=yes&screenshot=No&searchArea=No&captcha=yes';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just in case: is this accessible in China?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. They do support both traditional and simplified Chinese, and it looks like there are websites in China that use Freshdesk, so I think the answer is yes, but we'll definitely check.

@apple502j
Copy link
Contributor

Questions:

  1. Can email be optional somehow for logged-ins? Most Scratchers would use parents' emails, and the response should be readable easily without asking their parents. I have some ideas but the easiest would be "Reply by Alert".
  2. Is it possible to translate the form in the future?
  3. As I said above, forum link l10n would be a nice feature to have. Not all languages have their own forum and question topics, so it may be hard but I think it's worth.
  4. We should have more dropdown options: see Migrate - Contact Us #308
  5. What would happen if the iframe/js fails to load?

Using the Freshdesk Feedback form
* adds new component HelpForm for the Freshdesk component
* adds new contact-us view
* modifies Information page css with style for a header on the right hand nav section
* adds new route for the contact-us page (is identical to the old scratchr2 URL)
@chrisgarrity
Copy link
Contributor Author

@apple502j good questions.
This is just the beginning of a larger overhaul of the way that people contact us, so it's not going to do everything. For the particular questions:

  1. We could possibly pre-fill the email with whatever email is associated with their account, but it cannot be optional.
  2. Yes it will be able to be translated in the future
  3. Interesting idea, but probably out of scope for what we're doing right now.
  4. Next phase will be more dropdowns and required fields
  5. Right now it would just be a blank iframe if freshdesk is unavailable.

Decided that there is no need for the Nav sidebar since we don’t want the link to the Discussions right now. It could come back if we add the knowledge base.
Copy link
Contributor

@rschamp rschamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offline we discussed:

  • Passing this by the design team for input about what to do about the page width.
  • Increasing the height of the form to avoid the scroll-box inside a scrolling window issue, and to be transparent about the fields we're gathering

The other line comments I left are probably optional

Comment on lines 14 to 32
let subject = '';
let body = '';
const url = (window.location && window.location.search) || '';
// assumes that scratchr2 will only ever send one parameter
const params = url.split('?')[1];
if (typeof (params) !== 'undefined' && params.indexOf('studio') !== -1) {
subject = `Inappropriate content reported in studio ${params.split('=')[1]}`;
body = `https://scratch.mit.edu/studios/${params.split('=')[1]}`;
} else if (typeof (params) !== 'undefined' && params.indexOf('profile') !== -1) {
subject = `Inappropriate content reported in profile ${params.split('=')[1]}`;
body = `https://scratch.mit.edu/users/${params.split('=')[1]}`;
} else if (typeof (params) !== 'undefined' && params.indexOf('confirmation') !== -1) {
subject = 'Problem with email confirmation';
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be more readable and flexible if it happened outside of the component and resulted in passing in subject and body as props.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at other pages in www that use the search string the better approach looked like creating a constructor, and putting the values in the state.

src/components/helpform/helpform.jsx Show resolved Hide resolved
Scratchr2 has links to contact us from the report (profile|studio) dialogs. Looking at the scratchr2 source, there’s also one for confirmations, so this change handles the additional details in the same way - in the subject and description. In the future we could consider pre-filling specific form fields that we define.
* With Carl decided to put the sidebar back even with just one item. It’s still one we want to emphasize.
* made sure the freshdesk js file would not block loading the form (it doesn’t appear to be needed for the form, it’s probably mainly for the pop-up version)
* moved the query processing into a constructor (more idomatic react)
* expanded the form so it should not need to scroll - also noted that in incognito mode a captcha is shown, so created enough space for that to be visible.
Copy link
Contributor

@rschamp rschamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I have one comment about the iframe height, but not sure if that's a blocker or not.

<iframe
className="freshwidget-embedded-form"
frameBorder="0"
height="744px"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know you could include px for the height attribute — maybe React is stripping it out?

@rschamp rschamp added this to the March 2020 milestone Mar 9, 2020
@rschamp rschamp assigned chrisgarrity and unassigned rschamp Mar 9, 2020
@rschamp
Copy link
Contributor

rschamp commented Mar 9, 2020

We should also add to the test plan that we're going to look at availability from China.

@chrisgarrity chrisgarrity merged commit 9335274 into scratchfoundation:develop Mar 10, 2020
@chrisgarrity chrisgarrity deleted the freshdesk-contactus branch March 10, 2020 14:58
id="freshwidget-embedded-form"
scrolling="no"
src={`${prefix}&${title}&${username}&${browser}&${formSubject}&${formDescription}`}
title={<FormattedMessage id="contactUs.questionsForum" />}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is being rendered as [object Object] — I think we need to change it to use formatMessage (although I'm not sure what this attribute is here for).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants