Skip to content

Commit

Permalink
[Enhancement #62] Comments and Help features refactored
Browse files Browse the repository at this point in the history
  • Loading branch information
LaChope committed Oct 25, 2021
1 parent 344d3ed commit 757b027
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 85 deletions.
23 changes: 2 additions & 21 deletions src/components/Answer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,29 +146,10 @@ const Answer = (props) => {
/>
) : null;

const _renderQuestionCommentIcon = () => {
if (options.enableComments) {
return (
<Col className="no-padding-left" lg="auto">
<QuestionCommentIcon
question={question}
onChange={props.onCommentChange}
/>
</Col>
);
} else return null;
}

return (
<Row>
<Row lg="auto">
<Col className="no-padding-right" lg="auto">{label}</Col>
{questionHelp ?
<>
<Col className="no-padding-left" lg="1" >{questionHelp}</Col>
{_renderQuestionCommentIcon()}
</>
: _renderQuestionCommentIcon()
}
{props.icons}
</Row>
);
}
Expand Down
18 changes: 18 additions & 0 deletions src/components/IconsLayout.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from "react";
import {Col} from "react-bootstrap";
import PropTypes from "prop-types";

const IconsLayout = (props) => {
return (
React.Children.map(props.children, child => {
if (child) return <Col className={props.layout} lg="auto">{child}</Col>
return null;
})
);
}

IconsLayout.propTypes = {
layout: PropTypes.string.isRequired
}

export default IconsLayout;
62 changes: 33 additions & 29 deletions src/components/Question.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { CaretSquareUp, CaretSquareDown, InfoCircle } from '../styles/icons';
import { ConfigurationContext } from '../contexts/ConfigurationContext';
import classNames from 'classnames';
import QuestionCommentIcon from "./comment/QuestionCommentIcon";
import IconsLayout from "./IconsLayout";

// TODO Remove once the pretty layout is tested
const PRETTY_ANSWERABLE_LAYOUT = true;
Expand Down Expand Up @@ -155,15 +156,7 @@ export default class Question extends React.Component {
{label}
</h6>
</Col>
{this._renderQuestionHelp() ?
<>
<Col className="no-padding-left" lg="auto">
{this._renderQuestionHelp()}
</Col>
{this._renderQuestionComment()}
</>
: this._renderQuestionComment()
}
{this._renderIcons()}
</Row>
</Accordion.Toggle>
{collapsible ? <Accordion.Collapse>{cardBody}</Accordion.Collapse> : { cardBody }}
Expand Down Expand Up @@ -240,7 +233,9 @@ export default class Question extends React.Component {
answer={answers[i]}
question={question}
onChange={this.onAnswerChange}
onCommentChange={this.onCommentChange}/>
onCommentChange={this.onCommentChange}
icons={this._renderIcons()}
/>
</div>
{this._renderUnits()}
{this._renderPrefixes()}
Expand Down Expand Up @@ -320,30 +315,39 @@ export default class Question extends React.Component {
);
}

_renderQuestionHelp() {
const question = this.props.question;
let helpClass = FormUtils.isCheckbox(question) ? 'help-icon-checkbox' : 'help-icon-text-input';
if (FormUtils.isSection(question)) {
helpClass = 'help-icon-section';
static _renderQuestionHelp(question, intl) {
if (question[Constants.HELP_DESCRIPTION]) {
return <HelpIcon
text={JsonLdUtils.getLocalized(question[Constants.HELP_DESCRIPTION], intl)}/>
}
return question[Constants.HELP_DESCRIPTION] ? (
<HelpIcon
text={JsonLdUtils.getLocalized(question[Constants.HELP_DESCRIPTION], this.context.options.intl)}
iconClassContainer={helpClass}
/>
) : null;
return null;
}

_renderQuestionComment() {
if (this.context.options.enableComments) {
return (
<Col className="no-padding-left" lg="auto">
<QuestionCommentIcon
if (this.context.options.questionComments === "enable") {
return <QuestionCommentIcon
question={this.props.question}
onChange={this.onCommentChange} />
</Col>
);
} else return null;
onChange={this.onCommentChange}/>
}
return null;
}

_renderIcons() {
const question = this.props.question;
const intl = this.context.options.intl;
const renderQuestionHelp = Question._renderQuestionHelp(question, intl);
const renderQuestionComment = this._renderQuestionComment();

return (
<IconsLayout layout={Question.setIconsLayout()}>

This comment has been minimized.

Copy link
@blcham

blcham Oct 26, 2021

Collaborator

How about call it IconList component ?

{renderQuestionHelp}
{renderQuestionComment}
</IconsLayout>
);
}

static setIconsLayout() {

This comment has been minimized.

Copy link
@blcham

blcham Oct 26, 2021

Collaborator

How about passing this all the time ? and rename it, e.g. "icon-list" ...

If you make more specific implementation you can add new class so it will be e.g.
"icon-list" "resizable"
"icon-list" "wizard-friendly" :)) ---> "div .wizard .icon-list", ".icon-list"
"icon-list" "wizard-icon-list"

This comment has been minimized.

Copy link
@LaChope

LaChope Oct 26, 2021

Author Collaborator

Here is what I was thinking for the implementation:

  • The method
static setIconsClassname(classnameList) {
    let className = [];

    if (classnameList) {
      for (let i = 0; i < classnameList.length; i++) {
        className.push(" ");
        className.push(classnameList[i]);
      }
    }

    return Constants.ICONS_CLASSNAME.DEFAULT + className.toString().replace(",", "");
  }
  • How it is called (e.g. in WizardStep, while Question does not have any parameter):
        <IconList className={Question.setIconsClassname([Constants.ICONS_CLASSNAME.WIZARD])}>
          {renderQuestionHelp}
          {renderQuestionComment}
        </IconList>
  • In the CSS file:
.icon-list {
  padding-left: 0;
  background: green;
}

.icon-list.wizard-icon-list {
  background: red;
}
  • How it looks like:
    image

This comment has been minimized.

Copy link
@blcham

blcham Oct 26, 2021

Collaborator

It is little bit hard for me to follow.

I do not understand why do we even need Question.setIconsClassname.

Why dont just IconList have by default a className="icon-list".

And we would write for the question:

<IconList> 
{renderQuestionHelp} 
{renderQuestionComment} 
</IconList>

And for the wizard somethig like:

<IconList className="wizzard-icons"> 
{renderQuestionHelp} 
{renderQuestionComment} 
</IconList>

This comment has been minimized.

Copy link
@LaChope

LaChope Oct 26, 2021

Author Collaborator

Maybe I overcomplicated it, I wanted to have a method to be able to define a classname from pre-defined classnames and so styles (e.g. Constants.ICONS_CLASSNAME.DEFAULT => "icon-list"), to reuse it for other icons. But maybe it is a bit overkilled and/or not useful

This comment has been minimized.

Copy link
@blcham

blcham Oct 26, 2021

Collaborator

seems like that ;)

return "icon-layout";
}

_renderPrefixes() {
Expand Down
50 changes: 22 additions & 28 deletions src/components/wizard/WizardStep.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import {Button, ButtonToolbar, Card, Col, Row} from 'react-bootstrap';
import JsonLdUtils from 'jsonld-utils';
import PropTypes from 'prop-types';
import Constants from '../../constants/Constants';
import HelpIcon from '../HelpIcon';
import { FormQuestionsContext } from '../../contexts/FormQuestionsContext';
import Question from '../Question';
import QuestionCommentIcon from "../comment/QuestionCommentIcon";
import JsonLdObjectMap from "../../util/JsonLdObjectMap";
import IconsLayout from "../IconsLayout";


export default class WizardStep extends React.Component {
Expand All @@ -25,29 +25,14 @@ export default class WizardStep extends React.Component {
this.props.onPreviousStep();
};

_renderHelpIcon = () => {
_renderQuestionComment = () => {
const question = this.context.getFormQuestionsData([this.props.stepIndex]);
const comments = this.props.options.questionComments;

return question[Constants.HELP_DESCRIPTION] ? (
<HelpIcon
text={JsonLdUtils.getLocalized(question[Constants.HELP_DESCRIPTION], this.props.options.intl)}
iconClass="help-icon-section"
/>
) : null;
};

_renderQuestionCommentIcon = () => {
const question = this.context.getFormQuestionsData([this.props.stepIndex]);

if (this.props.options.enableComments) {
return (
<Col className="no-padding-left" lg="auto">
<QuestionCommentIcon
if (comments) {
return <QuestionCommentIcon
question={question}
onChange={this.onCommentChange}
/>
</Col>
);
onChange={this.onCommentChange}/>
} else return null;
}

Expand Down Expand Up @@ -84,6 +69,21 @@ export default class WizardStep extends React.Component {
this.context.updateFormQuestionsData(this.props.stepIndex || index, { ...this.props.step, ...change });
};

_renderIcons = () => {
const question = this.context.getFormQuestionsData([this.props.stepIndex]);
const intl = this.props.options.intl;

const renderQuestionHelp = Question._renderQuestionHelp(question, intl);
const renderQuestionComment = this._renderQuestionComment();

return (
<IconsLayout layout={Question.setIconsLayout()}>
{renderQuestionHelp}
{renderQuestionComment}
</IconsLayout>
);
}

render() {

const categoryClass = Question._getQuestionCategoryClass(this.props.step);
Expand All @@ -102,13 +102,7 @@ export default class WizardStep extends React.Component {
<Card.Header className="bg-primary text-white" as="h6" id={this.props.step['@id']}>
<Row>
<Col className="no-padding-right" lg="auto">{JsonLdUtils.getLocalized(this.props.step[JsonLdUtils.RDFS_LABEL], this.props.options.intl)}</Col>
{this._renderHelpIcon() ?
<>
<Col className="no-padding-left" lg="auto">{this._renderHelpIcon()}</Col>
{this._renderQuestionCommentIcon()}
</>
: this._renderQuestionCommentIcon()
}
{this._renderIcons()}
</Row>
</Card.Header>
<Card.Body className={categoryClass}>
Expand Down
7 changes: 1 addition & 6 deletions src/styles/s-forms.css
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ svg {
padding-right: 0;
}

.no-padding-left {
.icon-layout {

This comment has been minimized.

Copy link
@blcham

blcham Oct 26, 2021

Collaborator

maybe icon-list ?

padding-left: 0;
}

Expand All @@ -83,11 +83,6 @@ svg {
margin-left: -5px;
}

.help-icon-text-input {
margin-bottom: 25px;
bottom: 0;
}

.has-unit-label {
font-weight: bold;
margin-top: 12px;
Expand Down
3 changes: 2 additions & 1 deletion test/rendering/TestApp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ class TestApp extends React.Component {
{id: "http://fel.cvut.cz/people/max-chopart", label: "Max Chopart"},
{id: "http://fel.cvut.cz/people/miroslav-blasko", label: "Miroslav Blasko"}],
currentUser: "http://fel.cvut.cz/people/max-chopart",
enableComments: false
questionComments: "enable", // enable | disable | <onHover>
questionHelp: "enable" // <enable> | disable | onHover
};

return (
Expand Down

0 comments on commit 757b027

Please sign in to comment.