feat: show chat for desktop only

[varshamenon4]

Context

The Chatbot UI is conditionally displayed based on the screen width of a browser. The current breakpoint allows for mobile browsers in landscape mode to be able to view the chat toggle, although the tool itself has not been tested for mobile use yet.

The breakpoint should be changed to prevent mobile users from seeing the chat toggle. The updated breakpoint should only apply to the Chat toggle, not to any other triggers in the Learning MFE.

Changes

• Added new breakpoint specific to chat
• Set to min medium based on available breakpoints here: https://paragon-openedx.netlify.app/foundations/responsive/#available-breakpoints

Screenshots

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5d2aa5e) 88.35% compared to head (d894b35) 88.36%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1286   +/-   ##
=======================================
  Coverage   88.35%   88.36%           
=======================================
  Files         291      291           
  Lines        4938     4941    +3     
  Branches     1243     1245    +2     
=======================================
+ Hits         4363     4366    +3     
  Misses        561      561           
  Partials       14       14           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zacharis278]

I'll leave adding the test up to you, approved either way 👍

[varshamenon4]

Here is where I tried to create specialExams and add to testStore, but I'm thinking that this isn't the right way to add. Getting a bunch of console errors along the lines of:
Warning: Failed prop type: The prop unitIdis marked as required inBookmarkButton, but its value is undefined. at isBookmarked (/edx/app/frontend-app-learning/src/courseware/course/bookmark/BookmarkButton.jsx:27:3) at courseId (/edx/app/frontend-app-learning/src/courseware/course/sequence/Unit/index.jsx:18:3) at gated (/edx/app/frontend-app-learning/src/courseware/course/sequence/SequenceContent.jsx:13:3) at _classCallCheck (/edx/app/frontend-app-learning/node_modules/@edx/src/i18n/injectIntlWithShim.jsx:13:24) at injectIntl(ShimmedIntlComponent) at div at div at div at div at isGated (/edx/app/frontend-app-learning/node_modules/@edx/frontend-lib-special-exams/src/exam/Exam.jsx:24:3) at _classCallCheck (/edx/app/frontend-app-learning/node_modules/@edx/src/i18n/injectIntlWithShim.jsx:13:24) at injectIntl(ShimmedIntlComponent) at children (/edx/app/frontend-app-learning/node_modules/@edx/frontend-lib-special-exams/src/exam/ExamWrapper.jsx:15:24) at SequenceExamWrapper at div at unitId (/edx/app/frontend-app-learning/src/courseware/course/sequence/Sequence.jsx:32:3) at courseId (/edx/app/frontend-app-learning/src/courseware/course/sidebar/SidebarContextProvider.jsx:14:3) at courseId (/edx/app/frontend-app-learning/src/courseware/course/Course.jsx:24:3) at CourseWrapper (/edx/app/frontend-app-learning/src/courseware/course/Course.jsx:153:36) at children (/edx/app/frontend-app-learning/src/generic/user-messages/UserMessagesProvider.jsx:32:33) at basenameProp (/edx/app/frontend-app-learning/node_modules/react-router/lib/components.tsx:378:13) at basename (/edx/app/frontend-app-learning/node_modules/react-router-dom/index.tsx:354:3) at Provider (/edx/app/frontend-app-learning/node_modules/react-redux/lib/components/Provider.js:21:20) at store (/edx/app/frontend-app-learning/node_modules/@edx/src/react/OptionalReduxProvider.jsx:9:49) at _classCallCheck (/edx/app/frontend-app-learning/node_modules/@edx/src/react/ErrorBoundary.jsx:15:22) at IntlProvider (/edx/app/frontend-app-learning/node_modules/react-intl/src/components/provider.js:83:47) at store (/edx/app/frontend-app-learning/node_modules/@edx/src/react/AppProvider.jsx:47:39) at IntlProvider (/edx/app/frontend-app-learning/node_modules/react-intl/src/components/provider.js:83:47) at children (/edx/app/frontend-app-learning/src/setupTest.js:178:22)
The part that is most interesting is:
at isGated (/edx/app/frontend-app-learning/node_modules/@edx/frontend-lib-special-exams/src/exam/Exam.jsx:24:3) at _classCallCheck (/edx/app/frontend-app-learning/node_modules/@edx/src/i18n/injectIntlWithShim.jsx:13:24) at injectIntl(ShimmedIntlComponent) at children (/edx/app/frontend-app-learning/node_modules/@edx/frontend-lib-special-exams/src/exam/ExamWrapper.jsx:15:24) at SequenceExamWrapper
which is what makes me think this has to do with the special exams piece not being mocked correctly in my tests.

[michaelroytman]

I tend to find all of those console errors to be noise. I think they're related to the fact that a lot of component props correspond to pieces of Redux state, and before the API calls to hydrate the Redux store complete, these props are undefined, which causes prop type check failures.

Is the issue that this test is failing? I can see that in the Github actions. I'm not seeing a larger stack trace corresponding to that test, though, so I'm assuming it's just a matter of the assertion failing. Is there something I'm missing?

I believe that this is happening because of the changes we've made to Xpert. Xpert will now make an API call before it renders to fetch the data it needs to know whether it should render (i.e. whether it's enabled). Because of that, the Xpert component is not going to be in the DOM when you try to render the Course, because the API function has likely not completed. That's why there's no toggle-button in the DOM.

Ordinarily, you'd use the async functions of the React Testing Library to wait for the component to appear in your test. I'm not sure what's happening to that API call. It's not being mocked out in the test, so perhaps it's being made and floating out in the ether. Or maybe it's failing silently. Regardless, the Xpert component will likely never show up, even if you wait for it.

We actually ran into this same problem in Chat.test.jsx. I think your two options are:

1. Mock out the API call that Xpert is making. This is actually difficult, because the function that's making the API call is defined in another library (frontend-lib-special-exams). Based on some initial research, I think msw (Mock Service Worker) could work here, because you can specify the URL to mock and not the Javascript function that makes the API call, but I don't see it used in this repository.

2. Mock out the Chat component. This might feel a little odd. My rationale is that what your unit test is intending to test is the breakpoint - whether anything appears when the screen size is large enough, right? We don't actually care, for the purposes of the unit test, what appears. We've already tested every other part of the Xpert flow, so we don't need to actually test that what's appearing is Xpert. Take a look at how we've mocked out Xpert in the Chat test file. Notice how we mocked out the component with a dummy one. You could do something similar here. I think the choice is up to you whether to mock out Xpert or Chat. Chat is a pretty dumb component and doesn't do much, so I think it would be fine to mock it out, but maybe there is code in there that you still want to run as part of your test?

I hope that helps! Let me know if I've missed an important detail!

[varshamenon4]

This is so very helpful thank you! I think the instinct is right to just mock out the Chat component, especially since I just want to see that the screen size behavior is working correctly. I'll take a look at the Chat file for how to mock out a component.