Skip to content
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

fix BB-343 : Integrated the modification of identifiers and aliases into the primary editing workflow #1043

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6988878
other(editor): Introduced an identifier editing section and alias edi…
Tarunmeena0901 Dec 18, 2023
a56a519
typo fix
Tarunmeena0901 Dec 29, 2023
f9d3e89
typo fix
Tarunmeena0901 Dec 29, 2023
77bd98f
typo fix
Tarunmeena0901 Dec 29, 2023
8748dd2
typo fix
Tarunmeena0901 Dec 30, 2023
ddc6eb3
Merge branch 'BB-343' of https://github.com/Tarunmeena0901/bookbrainz…
Tarunmeena0901 Dec 30, 2023
a107325
Update alias-editor.js
Tarunmeena0901 Jan 2, 2024
68474e5
lint error fix
Tarunmeena0901 Jan 2, 2024
0ed84f8
fixed lint error
Tarunmeena0901 Jan 2, 2024
4b02088
identifier editing section heading change
Tarunmeena0901 Jan 12, 2024
8d6ad13
other(entity-editor): made UI changes and removed unecessary imports
Tarunmeena0901 Jan 16, 2024
ba28950
fix lint error
Tarunmeena0901 Jan 16, 2024
848b312
implemented new mockups and renamed identifierEditor and aliasEditor …
Tarunmeena0901 Apr 23, 2024
b5bb1d6
Merge remote-tracking branch 'upstream/master' into BB-343
Tarunmeena0901 Apr 23, 2024
e81bc36
Merge remote-tracking branch 'upstream/master' into BB-343
Tarunmeena0901 Apr 23, 2024
77ff0ff
fix error occur due to merge
Tarunmeena0901 Apr 23, 2024
3ef5b54
minor fix
Tarunmeena0901 Apr 24, 2024
7905072
build test fix
Tarunmeena0901 Apr 24, 2024
5f759c5
merging
Tarunmeena0901 Apr 24, 2024
107ad3d
Merge branch 'BB-343' of https://github.com/Tarunmeena0901/bookbrainz…
Tarunmeena0901 Apr 24, 2024
f645aa5
minor fix
Tarunmeena0901 Apr 24, 2024
eeaf8a5
minor fix
Tarunmeena0901 Apr 24, 2024
40a4066
done some remaining renaming of for alias merge
Tarunmeena0901 Apr 24, 2024
6bc94a3
lint error fix
Tarunmeena0901 Apr 24, 2024
2ec38cc
Merge remote-tracking branch 'origin/master' into pr/1043
MonkeyDo May 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 19 additions & 26 deletions src/client/entity-editor/alias-editor/alias-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

import {Button, Modal, OverlayTrigger, Tooltip} from 'react-bootstrap';

Check warning on line 18 in src/client/entity-editor/alias-editor/alias-editor.js

View workflow job for this annotation

GitHub Actions / ESLint

src/client/entity-editor/alias-editor/alias-editor.js#L18

'Modal' is defined but never used (@typescript-eslint/no-unused-vars)
import {hideAliasEditor, removeEmptyAliases} from './actions';

Check warning on line 19 in src/client/entity-editor/alias-editor/alias-editor.js

View workflow job for this annotation

GitHub Actions / ESLint

src/client/entity-editor/alias-editor/alias-editor.js#L19

'hideAliasEditor' is defined but never used (@typescript-eslint/no-unused-vars)
import AliasModalBody from './alias-modal-body';
import {FontAwesomeIcon} from '@fortawesome/react-fontawesome';
import PropTypes from 'prop-types';
Expand All @@ -25,7 +24,6 @@
import {connect} from 'react-redux';
import {faQuestionCircle} from '@fortawesome/free-solid-svg-icons';


/**
* Container component. The AliasEditor component contains a number of AliasRow
* elements, and renders these inside a modal, which appears when the show
Expand All @@ -37,15 +35,14 @@
* @param {Array} props.languageOptions - The list of possible languages for an
* alias.
* @param {Function} props.onClose - A function to be called when the button to
* close the editor is clicked.
* add new alias is clicked.
* @param {boolean} props.show - Whether or not the editor modal should be
* visible.
* @returns {ReactElement} React element containing the rendered AliasEditor.
*/
const AliasEditor = ({
languageOptions,
onClose,
show
onClose
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name onClose needs to be changed everywhere since the function has changed.
What does it do now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so this onClose is a dispatch function which is used to remove any empty alias row when user click on Done button (refer SS),
I thought to change the name to onDone but it doesn't seem very descriptive to me so I thought it may be good to hear some review

}) => {
const helpText = `Variant names for an entity such as alternate spelling, different script, stylistic representation, acronyms, etc.
Refer to the help page for more details and examples.`;
Expand All @@ -63,37 +60,33 @@
);

return (
<Modal show={show} size="lg" onHide={onClose}>
<Modal.Header>
<Modal.Title>
Alias Editor {helpIconElement}
</Modal.Title>
</Modal.Header>

<Modal.Body>
<div>
<div>
<div style={{display: 'flex'}}>
<h2>Add new alias</h2>
<div>
{helpIconElement}
</div>
</div>
</div>
<div>
<AliasModalBody languageOptions={languageOptions}/>
</Modal.Body>

<Modal.Footer>
<Button variant="primary" onClick={onClose}>Close</Button>
</Modal.Footer>
</Modal>
);
</div>
<div>
<Button variant="primary" onClick={onClose}>Done</Button>
</div>
</div>
);
};
AliasEditor.displayName = 'AliasEditor';
AliasEditor.propTypes = {
languageOptions: PropTypes.array.isRequired,
onClose: PropTypes.func.isRequired,
show: PropTypes.bool
};
AliasEditor.defaultProps = {
show: false
onClose: PropTypes.func.isRequired
};

function mapDispatchToProps(dispatch) {
return {
onClose: () => {
dispatch(hideAliasEditor());
dispatch(removeEmptyAliases());
}
};
Expand Down
4 changes: 2 additions & 2 deletions src/client/entity-editor/entity-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ const EntityEditor = (props: Props) => {
{heading}
</Card.Header>
<Card.Body>
<AliasEditor show={aliasEditorVisible} {...props}/>
<NameSection {...props}/>
<ButtonBar {...props}/>
{
Expand All @@ -97,7 +96,8 @@ const EntityEditor = (props: Props) => {
)
}
<RelationshipSection {...props}/>
<IdentifierEditor show={identifierEditorVisible} {...props}/>
<IdentifierEditor {...props}/>
<AliasEditor {...props}/>
<AnnotationSection {...props}/>
</Card.Body>
<Card.Footer>
Expand Down
43 changes: 18 additions & 25 deletions src/client/entity-editor/identifier-editor/identifier-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

import {Button, Modal, OverlayTrigger, Tooltip} from 'react-bootstrap';

Check warning on line 19 in src/client/entity-editor/identifier-editor/identifier-editor.js

View workflow job for this annotation

GitHub Actions / ESLint

src/client/entity-editor/identifier-editor/identifier-editor.js#L19

'Modal' is defined but never used (@typescript-eslint/no-unused-vars)
import {hideIdentifierEditor, removeEmptyIdentifiers} from './actions';

Check warning on line 20 in src/client/entity-editor/identifier-editor/identifier-editor.js

View workflow job for this annotation

GitHub Actions / ESLint

src/client/entity-editor/identifier-editor/identifier-editor.js#L20

'hideIdentifierEditor' is defined but never used (@typescript-eslint/no-unused-vars)
import {FontAwesomeIcon} from '@fortawesome/react-fontawesome';
import IdentifierModalBody from './identifier-modal-body';
import PropTypes from 'prop-types';
Expand All @@ -25,7 +25,6 @@
import {connect} from 'react-redux';
import {faQuestionCircle} from '@fortawesome/free-solid-svg-icons';


/**
* Container component. The IdentifierEditor component contains a number of
* IdentifierRow elements, and renders these inside a modal, which appears when
Expand All @@ -39,16 +38,13 @@
* @param {Function} props.onAddIdentifier - A function to be called when the
* button to add an identifier is clicked.
* @param {Function} props.onClose - A function to be called when the button to
* close the editor is clicked.
* @param {boolean} props.show - Whether or not the editor modal should be
* visible.
* add the identifier is clicked.
* @returns {ReactElement} React element containing the rendered
* IdentifierEditor.
*/
const IdentifierEditor = ({
identifierTypes,
onClose,
show
onClose
}) => {
const helpText = `identity of the entity in other databases and services, such as ISBN, barcode, MusicBrainz ID, WikiData ID, OpenLibrary ID, etc.
You can enter either the identifier only (Q2517049) or a full link (https://www.wikidata.org/wiki/Q2517049).`;
Expand All @@ -67,37 +63,34 @@
);

return (
<Modal show={show} size="lg" onHide={onClose}>
<Modal.Header>
<Modal.Title>
Identifier Editor {helpIconElement}
</Modal.Title>
</Modal.Header>
<div>
<div>
<div style={{display: 'flex'}}>
<h2>Add new indentifiers</h2>
Tarunmeena0901 marked this conversation as resolved.
Show resolved Hide resolved
<div>
{helpIconElement}
</div>
</div>
</div>

<Modal.Body>
<div>
<IdentifierModalBody identifierTypes={identifierTypes}/>
</Modal.Body>
</div>

<Modal.Footer>
<Button variant="primary" onClick={onClose}>Close</Button>
</Modal.Footer>
</Modal>
<div>
<Button variant="primary" onClick={onClose}>Done</Button>
</div>
</div>
);
};
IdentifierEditor.displayName = 'IdentifierEditor';
IdentifierEditor.propTypes = {
identifierTypes: PropTypes.array.isRequired,
onClose: PropTypes.func.isRequired,
show: PropTypes.bool
};
IdentifierEditor.defaultProps = {
show: false
onClose: PropTypes.func.isRequired
Tarunmeena0901 marked this conversation as resolved.
Show resolved Hide resolved
};

function mapDispatchToProps(dispatch) {
return {
onClose: () => {
dispatch(hideIdentifierEditor());
dispatch(removeEmptyIdentifiers());
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ function IdentifierRow({
<Col className="text-right" lg={{offset: 1, span: 3}}>
<Button
block
className="margin-top-d15"
className="margin-top-d22"
variant="danger"
onClick={onRemoveButtonClick}
>
Expand Down
Loading