Skip to content

Commit

Permalink
Fixes from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
Dananji committed Sep 1, 2023
1 parent a8f29ae commit bac6f83
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
14 changes: 12 additions & 2 deletions src/components/MarkersDisplay/MarkersDisplay.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const CancelIcon = () => {
);
};

const MarkersDisplay = ({ showHeading = true }) => {
const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
const { manifest, canvasIndex, playlist } = useManifestState();
const { player } = usePlayerState();
const manifestDispatch = useManifestDispatch();
Expand All @@ -107,6 +107,7 @@ const MarkersDisplay = ({ showHeading = true }) => {
}, [manifest, canvasIndex]);

const handleSubmit = (label, time, id, canvasId) => {
/* TODO:: Update the state once the API call is successful */
// Update markers in state for displaying in the player UI
let editedMarkers = playlistMarkers.map(m => {
if (m.id === id) {
Expand Down Expand Up @@ -139,18 +140,26 @@ const MarkersDisplay = ({ showHeading = true }) => {
// fetch(id, requestOptions)
// .then((response) => {
// console.log(response);
// /* Update state */
// });
// return;
};

const handleDelete = (id) => {
/* TODO:: Udate the state once the API call is successful */
// Update markers in state for displaying in the player UI
let remainingMarkers = playlistMarkers.filter(m => m.id != id);
setPlaylistMarkers(remainingMarkers);
manifestDispatch({ markers: remainingMarkers, type: 'setPlaylistMarkers' });

console.log('deleting marker: ', id);
// API call for DELETE
// fetch(id, requestOptions)
// .then((response) => {
// console.log(response);
// /* Update state */
// });
// return;
};

const handleMarkerClick = (e) => {
Expand All @@ -163,7 +172,7 @@ const MarkersDisplay = ({ showHeading = true }) => {
<div className="ramp--markers-display" data-testid="markers-display">
{showHeading && (
<div className="ramp--markers-display-title" data-testid="markers-display-title">
<h4>Markers</h4>
<h4>{headingText}</h4>
</div>
)}
<table>
Expand Down Expand Up @@ -358,6 +367,7 @@ const MarkerRow = ({ marker, handleSubmit, handleMarkerClick, handleDelete, isEd

MarkersDisplay.propTypes = {
showHeading: PropTypes.bool,
headingText: PropTypes.string,
};

export default MarkersDisplay;
Expand Down
1 change: 1 addition & 0 deletions src/components/MarkersDisplay/MarkersDisplay.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ This component reads manifest data from central state management provided by Con

`MarkersDisplay` component allows the following props;
- `showHeading`: accepts a Boolean value, which has a default value of `true` and is _not required_. This enables to hide the `Markers` heading on top of the component allowing to customize the user interface.
- `headingText`: accepts a String value, which has a default value of `Markers` and is _not required_. This value is used in the heading of the component, and enables to customize the text.

To import this component from the library;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ class VideoJSProgress extends vjsComponent {
this.state = { startTime: null, endTime: null };
this.times = options.targets[options.srcIndex];

/* When player is ready, call method to mount React component */
player.ready(() => {
});

player.on('loadedmetadata', () => {
this.mount();
this.setTimes();
Expand Down

0 comments on commit bac6f83

Please sign in to comment.