From 0c68fb3da5af9b11dbe1bd32585efc097e023994 Mon Sep 17 00:00:00 2001
From: thsparks <thsparks@microsoft.com>
Date: Mon, 16 Dec 2024 12:14:33 -0800
Subject: [PATCH] Show feedback comments on the sidebar

---
 theme/common.less                       |  78 ++++++++-
 theme/themes/pxt/globals/site.variables |   1 +
 webapp/src/app.tsx                      |   4 +
 webapp/src/editortoolbar.tsx            |  95 -----------
 webapp/src/feedbackPanel.tsx            | 205 ++++++++++++++++++++++++
 5 files changed, 281 insertions(+), 102 deletions(-)
 create mode 100644 webapp/src/feedbackPanel.tsx

diff --git a/theme/common.less b/theme/common.less
index 49f3ac692729..f7a33913b9bb 100644
--- a/theme/common.less
+++ b/theme/common.less
@@ -97,6 +97,10 @@ pre {
     overflow: visible;
 }
 
+.hasFeedback #maineditor {
+    right: @feedbackPanelWidth;
+}
+
 /* Editor tools */
 @editorToolsHeight: 10rem;
 @editorToolsCollapsedHeight: 4.7rem;
@@ -183,16 +187,76 @@ pre {
     cursor: pointer;
 }
 
-#feedbackNavArea {
-    margin: auto;
+#feedbackPanel {
+    display: flex;
+    flex-direction: column;
+    position: fixed;
+    top: @mainMenuHeight;
+    bottom: 4rem;
+    right: 0;
+    background-color: @filelistBackgroundColor;
+    z-index: @filelistZIndex;
+    font-family: @pageFont !important;
+    border-left: 2px solid #dadaef;
+    width: @feedbackPanelWidth;
 
-    .feedback-nav-container {
+    #feedbackNavArea {
+        margin: auto;
+        padding: 1rem;
+        border-bottom: 1px solid #dadaef;
+        width: 100%;
         display: flex;
-        flex-direction: row;
-        .feedback-nav-text {
-            font-size: 1.2rem;
-            font-weight: 600;
+        justify-content: center;
+
+        .feedback-nav-container {
+            display: flex;
+            flex-direction: row;
+            .feedback-nav-text {
+                font-size: 1.2rem;
+                font-weight: 600;
+                margin: 0.5rem;
+                display: flex;
+                justify-content: center;
+                align-items: center;
+            }
+            .feedback-nav-btn {
+                margin: 0 0.5rem;
+                padding: 0.2rem;
+            }
+        }
+    }
+
+    .feedback-comments-area {
+        display: flex;
+        flex-direction: column;
+        align-items: center;
+        justify-content: flex-start;
+        margin-top: 1rem;
+        gap: 1rem;
+        flex-grow: 1;
+
+        .feedback-comment-btn {
             margin: 0.5rem;
+            padding: 0;
+            width: 80%;
+            height: fit-content;
+            background-color: #c7f8ff;
+            border: 1px solid #00b4cc;
+            color: black;
+            border-radius: 0;
+            text-align: start;
+
+            .feedback-comment-header {
+                background-color: #00b4cc;
+                color: white;
+                font-weight: bold;
+                padding: 0.3rem 0.5rem;
+            }
+
+            .feedback-comment-text-area {
+                padding: 0.5rem;
+                text-wrap: auto;
+            }
         }
     }
 }
diff --git a/theme/themes/pxt/globals/site.variables b/theme/themes/pxt/globals/site.variables
index c291bef67ff2..541cc4330050 100644
--- a/theme/themes/pxt/globals/site.variables
+++ b/theme/themes/pxt/globals/site.variables
@@ -173,6 +173,7 @@
 @sideBarWidthLarge: 28rem;
 @simulatorWidth: 23rem;
 @simulatorWidthSmall: 20rem;
+@feedbackPanelWidth: 20rem;
 
 /*-------------------
    Background
diff --git a/webapp/src/app.tsx b/webapp/src/app.tsx
index eab841f53d32..020ceacf209d 100644
--- a/webapp/src/app.tsx
+++ b/webapp/src/app.tsx
@@ -82,6 +82,7 @@ import { Tour } from "./components/onboarding/Tour";
 import { parseTourStepsAsync } from "./onboarding";
 import { initGitHubDb } from "./idbworkspace";
 import { BlockDefinition, CategoryNameID } from "./toolbox";
+import { FeedbackPanel, getBlocksWithFeedback } from "./feedbackPanel";
 
 pxt.blocks.requirePxtBlockly = () => pxtblockly as any;
 pxt.blocks.requireBlockly = () => Blockly;
@@ -5275,6 +5276,7 @@ export class ProjectView
         const collapseIconTooltip = this.state.collapseEditorTools ? lf("Show the simulator") : lf("Hide the simulator");
         const isApp = cmds.isNativeHost() || pxt.BrowserUtils.isElectron();
         const hc = this.getData<boolean>(auth.HIGHCONTRAST)
+        const hasFeedback = getBlocksWithFeedback(this).length > 0;
 
         let rootClassList = [
             "ui",
@@ -5313,6 +5315,7 @@ export class ProjectView
             this.editor == this.textEditor && this.state.errorListState,
             'full-abs',
             pxt.appTarget.appTheme.embeddedTutorial ? "tutorial-embed" : "",
+            hasFeedback ? "hasFeedback" : "",
         ];
         this.rootClasses = rootClassList;
         const rootClasses = sui.cx(rootClassList);
@@ -5380,6 +5383,7 @@ export class ProjectView
                     {showCollapseButton && <sui.Button id='computertogglesim' className={`computer only collapse-button large`} icon={`inverted chevron ${showRightChevron ? 'right' : 'left'}`} title={collapseIconTooltip} onClick={this.toggleSimulatorCollapse} />}
                     {this.allEditors.map(e => e.displayOuter(editorOffset))}
                 </div>
+                {hasFeedback && <FeedbackPanel parent={this} />}
                 {inHome ? <div id="homescreen" className="full-abs">
                     <div className="ui home projectsdialog">
                         <header className="menubar" role="banner">
diff --git a/webapp/src/editortoolbar.tsx b/webapp/src/editortoolbar.tsx
index 6bc9ba3a7bc2..f04c3233c88e 100644
--- a/webapp/src/editortoolbar.tsx
+++ b/webapp/src/editortoolbar.tsx
@@ -443,9 +443,6 @@ export class EditorToolbar extends data.Component<ISettingsProps, EditorToolbarS
                         <identity.CloudSaveStatus headerId={header.id} />
                     </div>
                 </div>}
-            <div id="feedbackNavArea" role="menubar" className="ui column items">
-                <EditorToolbarFeedbackNav view={this.getViewString(computer)} parent={this.props.parent} />
-            </div>
             <div id="editorToolbarArea" role="menubar" className="ui column items">
                 {showUndoRedo && <div className="ui icon buttons">{this.getUndoRedo(computer)}</div>}
                 {showZoomControls && <div className="ui icon buttons mobile hide">{this.getZoomControl(computer)}</div>}
@@ -661,95 +658,3 @@ class EditorToolbarSaveInput extends React.Component<EditorToolbarSaveInputProps
         }
     }
 }
-
-interface EditorToolbarFeedbackNavProps {
-    view: string;
-    parent: pxt.editor.IProjectView;
-}
-interface EditorToolbarFeedbackNavState {
-    blocksWithFeedback: Blockly.Block[];
-    isBlocksActive: boolean;
-    currentIndex: number;
-}
-class EditorToolbarFeedbackNav extends React.Component<EditorToolbarFeedbackNavProps, EditorToolbarFeedbackNavState> {
-    constructor(props: EditorToolbarFeedbackNavProps) {
-        super(props);
-
-        this.state = {
-            isBlocksActive: this.props.parent.isBlocksActive(),
-            blocksWithFeedback: [],
-            currentIndex: 0, // TODO thsparks : This is getting set to NaN for some reason, even with this here??
-        }
-    }
-
-    protected handlePrevFeedbackClick = () => {
-        const newIndex = this.state.currentIndex > 0 ? this.state.currentIndex - 1 : this.state.blocksWithFeedback.length - 1;
-        this.setState({ currentIndex: newIndex });
-        this.focusOnBlockAtIndex(newIndex);
-    }
-
-    protected handleNextFeedbackClick = () => {
-        const currentIndex =  isNaN(this.state.currentIndex) ? 0 : this.state.currentIndex;
-        const newIndex = (currentIndex + 1) % this.state.blocksWithFeedback.length;
-        this.setState({ currentIndex: newIndex });
-        this.focusOnBlockAtIndex(newIndex);
-    }
-
-    protected focusOnBlockAtIndex(index: number) {
-        if (!this.props.parent.isBlocksActive()) return;
-
-        const block = this.state.blocksWithFeedback[index];
-        if (block) {
-            (this.props.parent.editor as blocks.Editor).editor.highlightBlock(block.id);
-            (this.props.parent.editor as blocks.Editor).editor.centerOnBlock(block.id);
-
-            block.getIcon(REVIEW_COMMENT_ICON_TYPE)?.setBubbleVisible(true);
-        }
-    }
-
-    protected getBlocksWithFeedback(): Blockly.Block[] {
-        if (!this.props.parent.isBlocksActive()) {
-            return [];
-        }
-
-        return this.props.parent.getBlocks().map(b => b as Blockly.Block).filter(b => !!ReviewCommentIcon.getReviewCommentForBlock(b));
-    }
-
-    protected updateBlocksWithFeedbackList() {
-        const blocksWithFeedback = this.getBlocksWithFeedback();
-        this.setState({ blocksWithFeedback, currentIndex: this.state.currentIndex % blocksWithFeedback.length });
-    }
-
-    componentDidMount() {
-        this.updateBlocksWithFeedbackList();
-    }
-
-    protected shouldUpdate(): boolean {
-        if (this.state.isBlocksActive !== this.props.parent.isBlocksActive()) return true;
-
-        const newBlocksWithFeedback = this.getBlocksWithFeedback();
-        if (newBlocksWithFeedback.length !== this.state.blocksWithFeedback.length) return true;
-        for (const blockId of newBlocksWithFeedback.map(b => b.id)) {
-            if (!this.state.blocksWithFeedback.find(b => b.id === blockId)) return true;
-        }
-
-        return false;
-    }
-
-    componentDidUpdate(prevProps: EditorToolbarFeedbackNavProps) {
-        if (this.shouldUpdate()) {
-            this.updateBlocksWithFeedbackList();
-
-            // Save this so we can tell if it changes.
-            this.setState({ isBlocksActive: this.props.parent.isBlocksActive() });
-        }
-    }
-
-    render() {
-        return this.state.blocksWithFeedback?.length ? (<div className="ui feedback-nav-container">
-                <EditorToolbarButton icon="arrow left" className="editortools-btn" title={lf("Previous Feedback")} onButtonClick={this.handlePrevFeedbackClick} view={this.props.view} />
-                <div className="feedback-nav-text">{lf("Feedback")}</div>
-                <EditorToolbarButton icon="arrow right" className="editortools-btn" title={lf("Next Feedback")} onButtonClick={this.handleNextFeedbackClick} view={this.props.view} />
-        </div>) : null;
-    }
-}
diff --git a/webapp/src/feedbackPanel.tsx b/webapp/src/feedbackPanel.tsx
new file mode 100644
index 000000000000..a46e05db5eb8
--- /dev/null
+++ b/webapp/src/feedbackPanel.tsx
@@ -0,0 +1,205 @@
+import * as React from "react";
+import * as Blockly from "blockly";
+import * as blocks from "./blocks";
+import { Button } from "../../react-common/components/controls/Button";
+import { REVIEW_COMMENT_ICON_TYPE, ReviewCommentIcon } from "../../pxtblocks/plugins/comments/reviewCommentIcon";
+
+
+
+export function getBlocksWithFeedback(projView: pxt.editor.IProjectView): Blockly.Block[] {
+    if (!projView.isBlocksActive()) {
+        return [];
+    }
+
+    return projView
+        .getBlocks()
+        .map((b) => b as Blockly.Block)
+        .filter((b) => !!ReviewCommentIcon.getReviewCommentForBlock(b));
+}
+
+interface FeedbackPanelProps {
+    parent: pxt.editor.IProjectView;
+}
+interface FeedbackPanelState {
+    blocksWithFeedback: Blockly.Block[];
+    isBlocksActive: boolean;
+    currentIndex: number;
+}
+export class FeedbackPanel extends React.Component<FeedbackPanelProps, FeedbackPanelState> {
+    constructor(props: FeedbackPanelProps) {
+        super(props);
+
+        this.state = {
+            isBlocksActive: this.props.parent.isBlocksActive(),
+            blocksWithFeedback: [],
+            currentIndex: 0, // TODO thsparks : This is getting set to NaN for some reason, even with this here??
+        };
+
+        this.focusOnBlockAtIndex = this.focusOnBlockAtIndex.bind(this);
+        this.handleNextFeedbackClick = this.handleNextFeedbackClick.bind(this);
+        this.handlePrevFeedbackClick = this.handlePrevFeedbackClick.bind(this);
+    }
+
+    protected handlePrevFeedbackClick = () => {
+        const newIndex =
+            this.state.currentIndex > 0 ? this.state.currentIndex - 1 : this.state.blocksWithFeedback.length - 1;
+        this.setState({ currentIndex: newIndex });
+        this.focusOnBlockAtIndex(newIndex);
+    };
+
+    protected handleNextFeedbackClick = () => {
+        const currentIndex = isNaN(this.state.currentIndex) ? 0 : this.state.currentIndex;
+        const newIndex = (currentIndex + 1) % this.state.blocksWithFeedback.length;
+        this.setState({ currentIndex: newIndex });
+        this.focusOnBlockAtIndex(newIndex);
+    };
+
+    protected focusOnBlockAtIndex(index: number) {
+        if (!this.props.parent.isBlocksActive()) return;
+
+        const block = this.state.blocksWithFeedback[index];
+        if (block) {
+            (this.props.parent.editor as blocks.Editor).editor.highlightBlock(block.id);
+            (this.props.parent.editor as blocks.Editor).editor.centerOnBlock(block.id);
+
+            block.getIcon(REVIEW_COMMENT_ICON_TYPE)?.setBubbleVisible(true);
+        }
+    }
+
+    protected getBlocksWithFeedback(): Blockly.Block[] {
+        return getBlocksWithFeedback(this.props.parent);
+    }
+
+    protected updateBlocksWithFeedbackList() {
+        const blocksWithFeedback = this.getBlocksWithFeedback();
+        this.setState({ blocksWithFeedback, currentIndex: this.state.currentIndex % blocksWithFeedback.length });
+    }
+
+    componentDidMount() {
+        this.updateBlocksWithFeedbackList();
+    }
+
+    protected shouldUpdate(): boolean {
+        if (this.state.isBlocksActive !== this.props.parent.isBlocksActive()) return true;
+
+        const newBlocksWithFeedback = this.getBlocksWithFeedback();
+        if (newBlocksWithFeedback.length !== this.state.blocksWithFeedback.length) return true;
+        for (const blockId of newBlocksWithFeedback.map((b) => b.id)) {
+            if (!this.state.blocksWithFeedback.find((b) => b.id === blockId)) return true;
+        }
+
+        return false;
+    }
+
+    componentDidUpdate(prevProps: EditorToolbarFeedbackNavProps) {
+        if (this.shouldUpdate()) {
+            this.updateBlocksWithFeedbackList();
+
+            // Save this so we can tell if it changes.
+            this.setState({ isBlocksActive: this.props.parent.isBlocksActive() });
+        }
+    }
+
+    render() {
+        return (
+            <div id="feedbackPanel" className="feedback-panel">
+                <div id="feedbackNavArea" role="menubar" className="ui column items">
+                    <EditorToolbarFeedbackNav
+                        parent={this.props.parent}
+                        focusOnBlockAtIndex={this.focusOnBlockAtIndex}
+                        blocksWithFeedback={this.state.blocksWithFeedback}
+                        isBlocksActive={this.state.isBlocksActive}
+                        handlePrevFeedbackClick={this.handlePrevFeedbackClick}
+                        handleNextFeedbackClick={this.handleNextFeedbackClick}
+                    />
+                </div>
+                <div id="feedbackPanelCommentsArea" className="feedback-comments-area">
+                    {this.state.blocksWithFeedback.map((block, index) => {
+                        const comment = ReviewCommentIcon.getReviewCommentForBlock(block);
+                        return (
+                            <InSidebarFeedbackComment
+                                key={index}
+                                parent={this.props.parent}
+                                commentText={comment}
+                                index={index}
+                                focusOnBlockAtIndex={this.focusOnBlockAtIndex}
+                            />
+                        );
+                    })}
+                </div>
+            </div>
+        );
+    }
+}
+
+interface InSidebarFeedbackCommentProps {
+    parent: pxt.editor.IProjectView;
+    commentText: string;
+    index: number;
+    focusOnBlockAtIndex: (index: number) => void;
+}
+interface InSidebarFeedbackCommentState {}
+class InSidebarFeedbackComment extends React.Component<InSidebarFeedbackCommentProps, InSidebarFeedbackCommentState> {
+    constructor(props: InSidebarFeedbackCommentProps) {
+        super(props);
+        this.state = {};
+    }
+
+    protected onClick = () => {
+        this.props.focusOnBlockAtIndex(this.props.index);
+    };
+
+    render() {
+        return (
+                <Button
+                    label={
+                        <div className="feedback-comment-container">
+                            <div className="feedback-comment-header">{lf("Feedback")}</div>
+                            <div className="feedback-comment-text-area">{this.props.commentText}</div>
+                        </div>
+                    }
+                    className="feedback-comment-btn"
+                    title={this.props.commentText}
+                    onClick={this.onClick}
+                />
+        );
+    }
+}
+
+interface EditorToolbarFeedbackNavProps {
+    parent: pxt.editor.IProjectView;
+    blocksWithFeedback: Blockly.Block[];
+    isBlocksActive: boolean;
+    handlePrevFeedbackClick: () => void;
+    handleNextFeedbackClick: () => void;
+    focusOnBlockAtIndex: (index: number) => void;
+}
+interface EditorToolbarFeedbackNavState {
+}
+class EditorToolbarFeedbackNav extends React.Component<EditorToolbarFeedbackNavProps, EditorToolbarFeedbackNavState> {
+    constructor(props: EditorToolbarFeedbackNavProps) {
+        super(props);
+    }
+
+    render() {
+        return this.props.blocksWithFeedback?.length ? (
+            <div className="ui feedback-nav-container">
+                <Button
+                    leftIcon="fas fa-arrow-left"
+                    // label={"<"}
+                    className="blue feedback-nav-btn"
+                    title={lf("Previous Feedback")}
+                    onClick={this.props.handlePrevFeedbackClick}
+                />
+                <div className="feedback-nav-text">{lf("Feedback")}</div>
+                <Button
+                    leftIcon="fas fa-arrow-right"
+                    // label={">"}
+                    className="blue feedback-nav-btn"
+                    title={lf("Next Feedback")}
+                    onClick={this.props.handleNextFeedbackClick}
+                />
+            </div>
+        ) : null;
+    }
+}