BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector #36

aayushagrawal9 · 2020-08-18T14:39:02Z

Implement components for forum selection (See mockup: https://edx.invisionapp.com/console/share/XDXMC3XU9MR/420742421)
JIRA tickets: https://openedx.atlassian.net/browse/OSPR-4902

Dependencies: None

Screenshots:

Mockup: https://edx.invisionapp.com/console/share/XDXMC3XU9MR/420742421

Sandbox URL: TBD - sandbox is being provisioned.

Merge deadline: None

Author notes and concerns:

This is a work in progress early PR

Reviewers

(@xitij2000)
edX reviewer[s] TBD

openedx-webhooks · 2020-08-18T14:39:08Z

Thanks for the pull request, @aayushagra! I've created BLENDED-550 to keep track of it in Jira.

When this pull request is ready, tag your edX technical lead.

xitij2000 · 2020-08-18T14:51:14Z

@aayushagra Could you add the [BD-03] tag to the title of this ticket, and make this a draft PR/ mark it as WIP.

src/forum-tool-selector-settings/ForumToolSelectorSettings.jsx

src/forum-tool-selector-settings/forumToolSelectorSettings.scss

xitij2000

This needs a lot of changes and restructuring. I've left some comments that should guide you along the way.

src/forum-tool-selector-settings/ForumToolSelectorSettings.jsx

frontend-app-course-authoring.code-workspace

src/forum-tool-selector-settings/ForumToolSelectorSettings.jsx

src/forum-tool-selector-settings/ForumToolSelectorSettings/ForumToolSelectorContainer.jsx

src/forum-tool-selector-settings/ForumToolSelectorSettings.jsx

src/forum-tool-selector-settings/ForumToolSelectorSettings/ForumToolSelectorContainer.jsx

todo

aayushagrawal9 · 2020-09-07T10:00:25Z

src/forum-tool-selector-settings/ForumToolSelectorSettings.jsx

+function chunk(arr, len) {
+  const chunks = [];
+  let i = 0;
+  const n = arr.length;
+  while (i < n) {
+    chunks.push(arr.slice(i, i += len));
+  }
+  return chunks;
+}


@xitij2000 Should this be moved elsewhere?

Lodash also has a chunk() function and lodash is already used by edx-platform so we can also use it here (Although it'll need to be added to package.json explicitly even though it's already installed otherwise the linter complains)

I don't know why you need this at all? This is simply not part of the design? The view isn't paginated.

@xitij2000 Because we can only fit 3 in a row. If there's more than 3 entries we need this to render the three cards + the features table below those three cards. Unless I've got the UI wrong for how to handle more than 3 cards?

Right now it's 3 cards, table for those 3 cards, then 3 more cards and their table etc

You can handle that using scroll snaps as I mentioned in my ticket description: https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-snap-type

You don't need javascript for this.

xitij2000 · 2020-09-07T12:42:21Z

src/forum-tool-selector-settings/ForumToolSelectorSettings.jsx

+function chunk(arr, len) {
+  const chunks = [];
+  let i = 0;
+  const n = arr.length;
+  while (i < n) {
+    chunks.push(arr.slice(i, i += len));
+  }
+  return chunks;
+}


I don't know why you need this at all? This is simply not part of the design? The view isn't paginated.

src/forum-tool-selector-settings/ForumToolSelectorSettings.jsx

src/forum-tool-selector-settings/features-table/FeaturesTable.jsx

src/forum-tool-selector-settings/forum-card/ForumCard.jsx

xitij2000

I think there are still a lot of adjustments that can be made to get this closer to the mockup. I've suggested some.

src/discussion-tool-selector/DiscussionToolSelector.scss

xitij2000 · 2020-09-17T10:32:32Z

src/discussion-tool-selector/DiscussionToolSelectorContainer.jsx

+  const [selectedForumId, setSelectedForumId] = useState(null);
+
+  const onSelectForum = (forumId) => {
+    setSelectedForumId(forumId);
+  };
+


Note: this will be replaced with redux state later.

...ssion-tool-selector/discussion-tool-selector/discussion-tool-option/DiscussionToolOption.jsx

src/discussion-tool-selector/discussion-tool-selector/features-table/FeaturesTable.jsx

xitij2000 · 2020-09-17T11:23:52Z

src/discussion-tool-selector/discussion-tool-selector/features-table/FeaturesTable.jsx

+          <tr>
+            <th>&nbsp;</th>
+            {forums.map(forum => (
+              <th className="text-center p-2" key={forum.forumId}>{forum.forumId}</th>


Suggested change

<th className="text-center p-2" key={forum.forumId}>{forum.forumId}</th>

<th className="text-center p-2" key={forum.forumId}><h5>{forum.forumId}</h5></th>

src/discussion-tool-selector/discussion-tool-selector/features-table/FeaturesTable.jsx

xitij2000

👍 I have a final few suggestions.

I tested this: tested on devstack. Checked that it matches the mockups, and forms the basis for upcoming work.
I read through the code
I checked for accessibility issues
Includes documentation

src/discussion-tool-selector/DiscussionToolSelector.scss

src/discussion-tool-selector/discussion-tool-selector/features-table/FeaturesTable.jsx

src/discussion-tool-selector/DiscussionToolSelector.scss

src/index.jsx

xitij2000 · 2020-09-21T07:56:45Z

@aayushagra Please squash your commits down to one, and add UI screenshots to the ticket description.

@stvstnfrd This PR is adding components for the forum selector UI.

aayushagrawal9 · 2020-10-06T05:59:15Z

@xitij2000 I have squashed the commits & added screenshots

davidjoy · 2021-01-08T19:07:55Z

This was merged as part of #44. Woohoo! Closing.

davidjoy · 2021-01-08T20:46:28Z

@natabene and @marcotuts - while this PR was "rejected" cause I closed it, the code was actually accepted and merged via a different PR which I rebased this on. I don't know if we want to account for this in our records on OSPRs, but I thought I'd mention it since it's not the first time this has happened. I want to make sure credit is given where credit is due. :)

natabene · 2021-01-09T21:30:07Z

@davidjoy Thanks for calling out, I defer to @marcotuts .

* feat: [2u-259] add components * feat: [2u-259] fix sidebar * feat: [2u-259] add tests, fix links * feat: [2u-259] fix messages * feat: [2u-159] fix reducer and sidebar * feat: [2u-259] fix reducer * feat: [2u-259] remove warning from selectors * feat: [2u-259] remove indents --------- Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local>

* feat: [2u-259] add components * feat: [2u-259] fix sidebar * feat: [2u-259] add tests, fix links * feat: [2u-259] fix messages * feat: [2u-159] fix reducer and sidebar * feat: [2u-259] fix reducer * feat: [2u-259] remove warning from selectors * feat: [2u-259] remove indents --------- Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local> feat: Course outline Status Bar (openedx#50) * feat: [2u-259] add components * feat: [2u-259] fix sidebar * feat: [2u-259] add tests, fix links * feat: [2u-259] fix messages * feat: [2u-159] fix reducer and sidebar * feat: add checklist * feat: [2u-259] fix reducer * feat: [2u-259] remove warning from selectors * feat: [2u-259] remove indents * feat: [2u-259] add api, enable modal * feat: [2u-259] add tests * feat: [2u-259] add translates * feat: [2u-271] fix transalates * feat: [2u-281] fix isQuery pending, utils, hooks * feat: [2u-281] fix useScrollToHashElement * feat: [2u-271] fix imports --------- Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local> feat: Course Outline Reindex (openedx#55) * feat: [2u-277] add alerts * feat: [2u-277] add translates * feat: [2u-277] fix tests * fix: [2u-277] fix slice and hook --------- Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local> fix: Course outline tests (openedx#56) * fix: fixed course outline status bar tests * fix: fixed course outline status bar tests * fix: fixed course outline enable highlights modal tests * fix: enable modal tests fix: increase code coverage on the page

* feat: Course outline Top level page (#36) * feat: [2u-259] add components * feat: [2u-259] fix sidebar * feat: [2u-259] add tests, fix links * feat: [2u-259] fix messages * feat: [2u-159] fix reducer and sidebar * feat: [2u-259] fix reducer * feat: [2u-259] remove warning from selectors * feat: [2u-259] remove indents --------- Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local> feat: Course outline Status Bar (#50) * feat: [2u-259] add components * feat: [2u-259] fix sidebar * feat: [2u-259] add tests, fix links * feat: [2u-259] fix messages * feat: [2u-159] fix reducer and sidebar * feat: add checklist * feat: [2u-259] fix reducer * feat: [2u-259] remove warning from selectors * feat: [2u-259] remove indents * feat: [2u-259] add api, enable modal * feat: [2u-259] add tests * feat: [2u-259] add translates * feat: [2u-271] fix transalates * feat: [2u-281] fix isQuery pending, utils, hooks * feat: [2u-281] fix useScrollToHashElement * feat: [2u-271] fix imports --------- Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local> feat: Course Outline Reindex (#55) * feat: [2u-277] add alerts * feat: [2u-277] add translates * feat: [2u-277] fix tests * fix: [2u-277] fix slice and hook --------- Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local> fix: Course outline tests (#56) * fix: fixed course outline status bar tests * fix: fixed course outline status bar tests * fix: fixed course outline enable highlights modal tests * fix: enable modal tests fix: increase code coverage on the page * refactor: improve course outline page feat: lms live link chore: update outline link fix: course outline link refactor: remove unnecessary css and rename test file refactor: remove unnecessary css from outlineSidebar test: make use of message variable instead of hardcoded text refactor: remove unnecessary h5 class test: use test id for detecting component refactor: update course outline url and some default messages --------- Co-authored-by: vladislavkeblysh <138868841+vladislavkeblysh@users.noreply.github.com>

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Aug 18, 2020

aayushagrawal9 changed the title ~~BB-2886 [Authoring MFE] Implement forum tool selector~~ BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector Aug 19, 2020

openedx-webhooks added blended PR is managed through 2U's blended developmnt program and removed open-source-contribution PR author is not from Axim or 2U labels Aug 19, 2020

aayushagrawal9 changed the title ~~BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector~~ [WIP] BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector Aug 19, 2020

aayushagrawal9 marked this pull request as draft August 19, 2020 07:34

aayushagrawal9 changed the title ~~[WIP] BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector~~ BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector Aug 19, 2020

xitij2000 requested changes Aug 19, 2020

View reviewed changes

xitij2000 requested changes Sep 3, 2020

View reviewed changes

xitij2000 reviewed Sep 4, 2020

View reviewed changes

todo Outdated Show resolved Hide resolved

aayushagrawal9 commented Sep 7, 2020

View reviewed changes

aayushagrawal9 marked this pull request as ready for review September 7, 2020 11:18

xitij2000 requested changes Sep 9, 2020

View reviewed changes

xitij2000 requested changes Sep 17, 2020

View reviewed changes

xitij2000 approved these changes Sep 18, 2020

View reviewed changes

Added new components for Discussion Tool Selection

63d1312

aayushagrawal9 force-pushed the aayush/add-forum-selector-tool branch from 334b98d to 63d1312 Compare October 6, 2020 05:53

davidjoy mentioned this pull request Jan 7, 2021

Discussion tool selection page skeleton #44

Merged

davidjoy closed this Jan 8, 2021

openedx-webhooks added rejected and removed needs triage labels Jan 8, 2021

xitij2000 deleted the aayush/add-forum-selector-tool branch August 12, 2021 04:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector #36

BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector #36

aayushagrawal9 commented Aug 18, 2020 •

edited

openedx-webhooks commented Aug 18, 2020 •

edited

xitij2000 commented Aug 18, 2020

xitij2000 left a comment

aayushagrawal9 Sep 7, 2020

xitij2000 Sep 7, 2020

aayushagrawal9 Sep 9, 2020

xitij2000 Sep 15, 2020

xitij2000 Sep 7, 2020

xitij2000 left a comment

xitij2000 Sep 17, 2020

xitij2000 Sep 17, 2020

xitij2000 left a comment •

edited

xitij2000 commented Sep 21, 2020

aayushagrawal9 commented Oct 6, 2020

davidjoy commented Jan 8, 2021

davidjoy commented Jan 8, 2021

natabene commented Jan 9, 2021

	<th className="text-center p-2" key={forum.forumId}>{forum.forumId}</th>
	<th className="text-center p-2" key={forum.forumId}><h5>{forum.forumId}</h5></th>

BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector #36

BB-2886 [BD-03] [Authoring MFE] Implement forum tool selector #36

Conversation

aayushagrawal9 commented Aug 18, 2020 • edited

openedx-webhooks commented Aug 18, 2020 • edited

xitij2000 commented Aug 18, 2020

xitij2000 left a comment

Choose a reason for hiding this comment

aayushagrawal9 Sep 7, 2020

Choose a reason for hiding this comment

xitij2000 Sep 7, 2020

Choose a reason for hiding this comment

aayushagrawal9 Sep 9, 2020

Choose a reason for hiding this comment

xitij2000 Sep 15, 2020

Choose a reason for hiding this comment

xitij2000 Sep 7, 2020

Choose a reason for hiding this comment

xitij2000 left a comment

Choose a reason for hiding this comment

xitij2000 Sep 17, 2020

Choose a reason for hiding this comment

xitij2000 Sep 17, 2020

Choose a reason for hiding this comment

xitij2000 left a comment • edited

Choose a reason for hiding this comment

xitij2000 commented Sep 21, 2020

aayushagrawal9 commented Oct 6, 2020

davidjoy commented Jan 8, 2021

davidjoy commented Jan 8, 2021

natabene commented Jan 9, 2021

aayushagrawal9 commented Aug 18, 2020 •

edited

openedx-webhooks commented Aug 18, 2020 •

edited

xitij2000 left a comment •

edited