Add extension buttons (simplified variant) #53
Conversation
| @@ -105,13 +106,34 @@ class Record extends React.Component { | |||
| const {record, recordSaved, formgen} = this.props; | |||
|
|
|||
| return <div className="mt-3 text-center"> | |||
| <Button variant='success' size='sm' | |||
| {record.phase === RECORD_PHASE.COMPLETED | |||
There was a problem hiding this comment.
@LaChope why are we using different mechanism to hide buttons as couple lines bellow where we use hidden attribute? I.e.:
<Button className="mx-1" variant='success' size='sm'
disabled={formgen.status === ACTION_STATUS.PENDING || recordSaved.status === ACTION_STATUS.PENDING
|| !this.state.isFormValid || !record.state.isComplete()}
hidden={(record.phase === RECORD_PHASE.COMPLETED && !this._isAdmin()) ||
record.phase === RECORD_PHASE.PUBLISHED}
onClick={this.props.handlers.onSave}>
{this.i18n('save')}{recordSaved.status === ACTION_STATUS.PENDING && <LoaderSmall/>}
</Button>
There was a problem hiding this comment.
@blcham for consistency, the mechanism should be the same everywhere, I just forgot to change it in all places. It should be like in line 109{record.phase === RECORD_PHASE.COMPLETED for testing reasons. When using the hidden prop, the buttons are still present in the DOM, and thus breaking the tests.
| @@ -7,7 +7,7 @@ import Records from "../../../js/components/record/Records"; | |||
| import {ACTION_STATUS, ROLE} from "../../../js/constants/DefaultConstants"; | |||
| import enLang from '../../../js/i18n/en'; | |||
|
|
|||
| describe('Records', function () { | |||
| describe.skip('Records', function () { | |||
There was a problem hiding this comment.
We need to fix failing tests and enable it !
There was a problem hiding this comment.
I thought I did it everywhere. I see you already created issue in #54
| @@ -10,7 +10,7 @@ import enLang from '../../../js/i18n/en'; | |||
|
|
|||
| jest.mock("../../../js/components/record/TypeaheadAnswer", () =>() => <input />); | |||
|
|
|||
| describe('Record', function () { | |||
| describe.skip('Record', function () { | |||
There was a problem hiding this comment.
We need to fix failing tests and enable it !
There was a problem hiding this comment.
Why not just change failing test in a way that no extension will be expected.
| @@ -110,6 +110,7 @@ describe('Record', function () { | |||
| } | |||
| }); | |||
|
|
|||
| //TODO (after migrating to React 18): Create test cases for different extension (operator and supplier cases) | |||
There was a problem hiding this comment.
I would put this rather to ticket and not here.
There was a problem hiding this comment.
If we fix tests we will know right away which tests are using which extensions.
There was a problem hiding this comment.
Ok thanks, it will be dealt in #54
| @@ -10,7 +10,7 @@ import enLang from '../../../js/i18n/en'; | |||
|
|
|||
| jest.mock("../../../js/components/record/TypeaheadAnswer", () =>() => <input />); | |||
|
|
|||
| describe('Record', function () { | |||
| describe.skip('Record', function () { | |||
There was a problem hiding this comment.
Why not just change failing test in a way that no extension will be expected.
This is same PR as #22 but rebased onto main to make cleaner history.