Skip to content

Commit

Permalink
chore: Minor fixes & improvements (ComPlat#2232)
Browse files Browse the repository at this point in the history
* Do not modify Sample object when rendering ElementalCompositionCustom

Assigning to `el_composition.data` would cause the sample to be
considered as `isEdited`.

* Add missing key prop when rendering UserLabels

* Do not update state during render in InboxModal

Before this change, we would `this.state.sortColumn = ...` during render
in order to get the latest value from UserStore. Now we properly listen
(and unlisten) for store changes and update the state using
`this.setState`.

With this change we also set the initial UIStore state when the
component mounts and not only when the store state changes.

* Remove unused genericEls prop from ManagingActions

The ManagingActions component handles syncing genericEls with UserStore
itself. No need to pass that information in as prop.

* Do not copy UIStore state into local component state

We used to copy the state to determine whether the selection had changed
or not. However, we can simply calculate that information from the
UIStore state without the local state.

* Fix showing/hiding preview images in list of Samples

* Remove unnecessary UIAction.selectSyncCollection

This used to trigger `UIState.handleSelectSyncCollection` which
forwarded to `UIStore.handleSelectCollection`. Removing in favour of
less indirection.

* Use currentTab data stored in UserStore

Before this change, we would always reset the currentTab to 0 and not
use the data stored in the UserStore.

This was fine when we never unmounted/mounted the ElementList component
but leads to the undesired effect of not respecting the user's selected
tab when we unmount/mount the component.

* Remove obsolete `update` prop in ResearchPlanDetails

This was used to force a re-render when changing to full screen. It does
not seem to be necessary anymore.

* Prefer setState over modifying state directly

Also fix collection type. JS Array has no `size` property.

* Apply store state when ElementsTable mounts

* Use setState instead of mutating state directly in ElementsTable
  • Loading branch information
maiwald authored Nov 14, 2024
1 parent 2e624f7 commit d5621b2
Show file tree
Hide file tree
Showing 17 changed files with 117 additions and 219 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,11 @@ export default class ResearchPlanDetails extends Component {
const { researchPlan } = props;
this.state = {
researchPlan,
update: false,
visible: Immutable.List(),
currentUser: (UserStore.getState() && UserStore.getState().currentUser) || {},
};
this.handleSwitchMode = this.handleSwitchMode.bind(this);
this.handleResearchPlanChange = this.handleResearchPlanChange.bind(this);
this.toggleFullScreen = this.toggleFullScreen.bind(this);
this.handleNameChange = this.handleNameChange.bind(this);
this.handleBodyChange = this.handleBodyChange.bind(this);
this.handleBodyAdd = this.handleBodyAdd.bind(this);
Expand Down Expand Up @@ -261,13 +259,6 @@ export default class ResearchPlanDetails extends Component {
this.setState({ visible });
}

toggleFullScreen() {
this.props.toggleFullScreen();

// toogle update prop to notify react data grid for view change
this.setState({ update: !this.state.update });
}

dropWellplate(wellplate) {
const { researchPlan } = this.state;
researchPlan.changed = true;
Expand Down Expand Up @@ -351,7 +342,7 @@ export default class ResearchPlanDetails extends Component {
);
}

renderResearchPlanMain(researchPlan, update) { /* eslint-disable react/jsx-no-bind */
renderResearchPlanMain(researchPlan) { /* eslint-disable react/jsx-no-bind */
const {
name, body, changed, attachments
} = researchPlan;
Expand Down Expand Up @@ -434,7 +425,6 @@ export default class ResearchPlanDetails extends Component {
onDelete={this.handleBodyDelete.bind(this)}
onExport={this.handleExportField.bind(this)}
onCopyToMetadata={this.handleCopyToMetadata.bind(this)}
update={update}
edit={edit}
copyableFields={[
{ title: 'Subject', fieldName: 'subject' },
Expand Down Expand Up @@ -515,7 +505,7 @@ export default class ResearchPlanDetails extends Component {
</Button>
</OverlayTrigger>
<OverlayTrigger placement="bottom" overlay={<Tooltip id="fullSample">Full Research Plan</Tooltip>}>
<Button variant="info" size="xxsm" onClick={this.toggleFullScreen}>
<Button variant="info" size="xxsm" onClick={this.props.toggleFullScreen}>
<i className="fa fa-expand" aria-hidden="true" />
</Button>
</OverlayTrigger>
Expand All @@ -529,15 +519,15 @@ export default class ResearchPlanDetails extends Component {
}

render() {
const { researchPlan, update, visible } = this.state;
const { researchPlan, visible } = this.state;

const tabContentsMap = {
research_plan: (
<Tab eventKey="research_plan" title="Research plan" key={`rp_${researchPlan.id}`}>
{
!researchPlan.isNew && <CommentSection section="research_plan_research_plan" element={researchPlan} />
}
{this.renderResearchPlanMain(researchPlan, update)}
{this.renderResearchPlanMain(researchPlan)}
<PrivateNoteElement element={researchPlan} disabled={researchPlan.can_update} />
</Tab>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Field from 'src/apps/mydb/elements/details/researchPlans/researchPlanTab/
export default class ResearchPlanDetailsBody extends Component {
render() {
const {
body, disabled, onChange, onDrop, onAdd, onDelete, onExport, update, edit, isNew,
body, disabled, onChange, onDrop, onAdd, onDelete, onExport, edit, isNew,
copyableFields, onCopyToMetadata
} = this.props;

Expand All @@ -29,7 +29,6 @@ export default class ResearchPlanDetailsBody extends Component {
onDelete={onDelete.bind(this)}
onExport={onExport.bind(this)}
onCopyToMetadata={onCopyToMetadata.bind(this)}
update={update}
edit={edit}
tableIndex={tableIndex}
isNew={isNew}
Expand All @@ -49,7 +48,6 @@ export default class ResearchPlanDetailsBody extends Component {
onDelete={onDelete.bind(this)}
onExport={onExport.bind(this)}
onCopyToMetadata={onCopyToMetadata.bind(this)}
update={update}
edit={edit}
tableIndex={tableIndex}
isNew={isNew}
Expand Down Expand Up @@ -92,7 +90,6 @@ ResearchPlanDetailsBody.propTypes = {
onDelete: PropTypes.func,
onExport: PropTypes.func,
onCopyToMetadata: PropTypes.func,
update: PropTypes.bool,
edit: PropTypes.bool,
isNew: PropTypes.bool,
copyableFields: PropTypes.arrayOf(PropTypes.object),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import ResearchPlanDetailsFieldReaction from 'src/apps/mydb/elements/details/res
export default class ResearchPlanDetailsField extends Component {
render() {
const {
field, index, disabled, onChange, onDrop, onDelete, onExport, update, edit, tableIndex,
field, index, disabled, onChange, onDrop, onDelete, onExport, edit, tableIndex,
onCopyToMetadata, isNew, copyableFields
} = this.props;
let label;
Expand Down Expand Up @@ -82,7 +82,6 @@ export default class ResearchPlanDetailsField extends Component {
disabled={disabled}
onChange={onChange.bind(this)}
onExport={onExport}
update={update}
edit={edit}
tableIndex={tableIndex}
/>
Expand Down Expand Up @@ -206,7 +205,6 @@ ResearchPlanDetailsField.propTypes = {
onCopyToMetadata: PropTypes.func,
isNew: PropTypes.bool,
copyableFields: PropTypes.arrayOf(PropTypes.object),
update: PropTypes.bool,
edit: PropTypes.bool,
attachments: PropTypes.array
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export default class ResearchPlanDetailsFieldTable extends Component {
constructor(props) {
super(props);
this.state = {
update: this.props.update,
currentlyCollapsedInEditMode: this.props?.field?.value?.startCollapsed ?? false,
currentlyCollapsedInViewMode: this.props?.field?.value?.startCollapsed ?? false,
columnNameModal: {
Expand All @@ -45,12 +44,6 @@ export default class ResearchPlanDetailsFieldTable extends Component {
this.ref = React.createRef();
}

componentDidUpdate() {
if (this.state.update !== this.props.update) {
this.setState({ update: this.props.update });
}
}

buildColumn(columnName) {
return {
cellEditor: 'agTextCellEditor',
Expand Down Expand Up @@ -663,6 +656,5 @@ ResearchPlanDetailsFieldTable.propTypes = {
index: PropTypes.number,
disabled: PropTypes.bool,
onChange: PropTypes.func,
update: PropTypes.bool,
edit: PropTypes.bool
};
Original file line number Diff line number Diff line change
Expand Up @@ -32,33 +32,23 @@ export default class ElementalCompositionCustom extends React.Component {
}

elementsList(el_composition, concat_formula) {
const elements = [];
const newData = {};

// be sure that 3, 2-symbol (Br) elements are all before one-symbol (B)!
// TODO: check performance
const mendeleev = /(Uut|Uup|Uus|Uuo|He|Li|Be|Ne|Na|Mg|Al|Si|Cl|Ar|Ca|Sc|Ti|Cr|Mn|Fe|Co|Ni|Cu|Zn|Ga|Ge|As|Se|Br|Kr|Rb|Sr|Zr|Nb|Mo|Tc|Ru|Rh|Pd|Ag|Cd|In|Sn|Sb|Te|Xe|Cs|Ba|La|Ce|Pr|Nd|Pm|Sm|Eu|Gd|Tb|Dy|Ho|Er|Tm|Yb|Lu|Hf|Ta|Re|Os|Ir|Pt|Au|Hg|Tl|Pb|Bi|Po|At|Rn|Fr|Ra|Ac|Th|Pa|Np|Pu|Am|Cm|Bk|Cf|Es|Fm|Md|No|Lr|Rf|Db|Sg|Bh|Hs|Mt|Ds|Rg|Cn|Fl|Lv|H|B|C|N|O|F|P|S|K|V|Y|I|W|U)/g;
const keys = _.uniq(concat_formula.match(mendeleev)).sort();

// add new key to custom composition, so that we have new input
keys.forEach((key) => {
newData[key] = (el_composition.data[key] || 0.0);
elements.push(
<Form.Group key={key} className="d-flex align-items-baseline gap-2">
<Form.Label>{key}</Form.Label>
<NumeralInput
numeralFormat="0,0.00"
label={key}
value={newData[key]}
defaultValue={newData[key]}
onChange={(v) => this.handleElementsListChanged(v, key, el_composition)}
/>
</Form.Group>
);
});

el_composition.data = newData;
return elements;
return keys.map((key) => (
<Form.Group key={key} className="d-flex align-items-baseline gap-2">
<Form.Label>{key}</Form.Label>
<NumeralInput
numeralFormat="0,0.00"
label={key}
value={el_composition.data[key] || 0.0}
onChange={(v) => this.handleElementsListChanged(v, key, el_composition)}
/>
</Form.Group>
));
}

render() {
Expand Down
44 changes: 21 additions & 23 deletions app/packs/src/apps/mydb/elements/list/ElementsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,39 +99,35 @@ export default class ElementsList extends React.Component {
onChangeUser(state) {
let visible = Immutable.List();
let hidden = Immutable.List();
let currentTabIndex = 0;

const { currentType } = state;
let type = state.currentType;
let { currentType, currentTab } = state;

if (typeof (state.profile) !== 'undefined' && state.profile
&& typeof (state.profile.data) !== 'undefined' && state.profile.data) {
visible = getArrayFromLayout(state.profile.data.layout, true);
hidden = getArrayFromLayout(state.profile.data.layout, false);
currentTabIndex = visible.findIndex((e) => e === currentType);
if (type === '') { type = visible.get(0); }
currentTab = visible.findIndex((e) => e === currentType);
if (currentType === '') { currentType = visible.get(0); }
}
if (hidden.size === 0) {
hidden = ArrayUtils.pushUniq(hidden, 'hidden');
}

if (currentTabIndex < 0) currentTabIndex = 0;
if (currentTab < 0) currentTab = 0;

if (typeof type !== 'undefined' && type != null) {
KeyboardActions.contextChange.defer(type);
if (typeof currentType !== 'undefined' && currentType != null) {
KeyboardActions.contextChange.defer(currentType);
}

this.setState({
currentTab: currentTabIndex,
genericEls: state.genericEls || [],
currentTab,
visible,
hidden
});
}

onChangeUI(state) {
const { totalCheckedElements } = this.state;
let forceUpdate = false;
// const genericNames = (genericEls && genericEls.map(el => el.name)) || [];
let genericKlasses = [];
const currentUser = (UserStore.getState() && UserStore.getState().currentUser) || {};
Expand All @@ -141,31 +137,33 @@ export default class ElementsList extends React.Component {
}
const elNames = ['sample', 'reaction', 'screen', 'wellplate', 'research_plan', 'cell_line'].concat(genericKlasses);

const newTotalCheckedElements = {};
let needsUpdate = false;
elNames.forEach((type) => {
const elementUI = state[type] || {
checkedAll: false, checkedIds: [], uncheckedIds: [], currentId: null
checkedAll: false,
checkedIds: Immutable.List(),
uncheckedIds: Immutable.List(),
};
const element = ElementStore.getState().elements[`${type}s`];
const nextCount = elementUI.checkedAll
? (element.totalElements - elementUI.uncheckedIds.size)
: elementUI.checkedIds.size;
if (!forceUpdate && nextCount !== (totalCheckedElements[type] || 0)) { forceUpdate = true; }
totalCheckedElements[type] = nextCount;
needsUpdate = needsUpdate || nextCount !== totalCheckedElements[type];
newTotalCheckedElements[type] = nextCount
});

this.setState((previousState) => ({ ...previousState, totalCheckedElements }));
// could not use shouldComponentUpdate because state.totalCheckedElements
// has already changed independently of setstate
if (forceUpdate) { this.forceUpdate(); }
if (needsUpdate) {
this.setState({ totalCheckedElements: newTotalCheckedElements });
}
}

handleRemoveSearchResult(searchStore) {
searchStore.changeShowSearchResultListValue(false);
UIActions.clearSearchById();
ElementActions.changeSorting(false);
const { currentCollection, isSync } = UIStore.getState();
isSync ? UIActions.selectSyncCollection(currentCollection)
: UIActions.selectCollection(currentCollection);
const { currentCollection } = UIStore.getState();
UIActions.selectCollection(currentCollection);
}

handleTabSelect(tab) {
Expand All @@ -186,7 +184,7 @@ export default class ElementsList extends React.Component {

render() {
const {
visible, hidden, totalCheckedElements, totalElements
visible, hidden, totalCheckedElements, totalElements, currentTab
} = this.state;
const { overview } = this.props;

Expand Down Expand Up @@ -262,7 +260,7 @@ export default class ElementsList extends React.Component {
<div className="position-relative">
<Tabs
id="tabList"
defaultActiveKey={0}
activeKey={currentTab}
onSelect={(eventKey) => this.handleTabSelect(parseInt(eventKey, 10))}
>
{tabItems}
Expand Down
45 changes: 24 additions & 21 deletions app/packs/src/apps/mydb/elements/list/ElementsTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,32 @@ export default class ElementsTable extends React.Component {
}

componentDidMount() {
UIStore.getState();
ElementStore.listen(this.onChange);
this.onChange(ElementStore.getState());

UIStore.listen(this.onChangeUI);
this.initState();
this.onChangeUI(UIStore.getState());

const { type, genericEl } = this.props;
if (type === 'reaction' || genericEl) {
const userState = UserStore.getState();
const filters = userState.profile.data.filters || {};

const { elementsGroup, elementsSort, sortDirection } = this.state;
const newElementsGroup = filters[type]?.group || 'none';
const newElementsSort = filters[type]?.sort ?? true;
const newSortDirection = filters[type]?.direction || 'DESC';

if (newElementsGroup !== elementsGroup
|| newElementsSort !== elementsSort
|| newSortDirection !== sortDirection) {
this.setState({
elementsGroup: newElementsGroup,
elementsSort: newElementsSort,
sortDirection: newSortDirection,
});
}
}
}

componentWillUnmount() {
Expand Down Expand Up @@ -169,25 +191,6 @@ export default class ElementsTable extends React.Component {
if (toDate !== date) UIActions.setToDate(date);
}

initState = () => {
this.onChange(ElementStore.getState());

const { type, genericEl } = this.props;

if (type === 'reaction' || genericEl) {
const userState = UserStore.getState();
const filters = userState.profile.data.filters || {};

// you are not able to use this.setState because this would rerender it again and again ...
// eslint-disable-next-line react/no-direct-mutation-state
this.state.elementsGroup = filters[type]?.group || 'none';
// eslint-disable-next-line react/no-direct-mutation-state
this.state.elementsSort = filters[type]?.sort ?? true;
// eslint-disable-next-line react/no-direct-mutation-state
this.state.sortDirection = filters[type]?.direction || 'DESC';
}
};

changeCollapse = (collapseAll) => {
this.setState({ collapseAll: !collapseAll });
};
Expand Down
Loading

0 comments on commit d5621b2

Please sign in to comment.