-
Notifications
You must be signed in to change notification settings - Fork 49
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
745 add order parameter #790
745 add order parameter #790
Conversation
* pass order value to Section, Page, and Question
* pass order value to QuestionSet * pass order value to Question from QuestionSet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @CalamityC ! Thanks for the patch. Could you disable your autoformatter for RDMO. I do not care much on how you format the code but the automatic changes clutter the diff. (We had a long discussion about autoformatting the python part code and we decided (i.e. I insisted) that we don't autoformat for now.)
<code className={className}>{uri}</code> | ||
</Link> | ||
</Link>{order !== undefined && order !== null ? <code className="code-order ng-binding">{order}</code> : null}</> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there should be a {' '}
between the URI and the order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use lodash's !isEmpty(order)
here (import isEmpty from 'lodash/isEmpty'
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use lodash's
!isEmpty(order)
here (import isEmpty from 'lodash/isEmpty'
).
Unfortunately isEmpty
does not work with numbers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right! Its isNil
https://lodash.com/docs/#isNil
* fix change requests
d8dbae1
to
6cc0dbd
Compare
Hey @CalamityC I just saw that questions don't have the order visible, was that on oversight or on purpose. I think it would be nice to have the parameter visible there as well. |
Hi @jochenklar I added it to |
Ah sorry, I got it wrong. Its not in the nested section, page and question set view. But maybe nobody uses them anyway. |
Description
Related issue: #745
Motivation and Context
How has this been tested?
Manually and locally in the UI
Screenshots (if appropriate)
Types of Changes
Checklist