benwiley4000 / cassette Goto Github PK

Hey,

are you planning to add a volume control?

Readme should have a section on compiling styles with SASS similar to react-gif-player's.

ya

The createAudioElementWithLoopEvent module declares at its top level:

const loopchange = new Event('loopchange');

This is an issue in Node since Event is a client-side class. Importing this module will throw: ReferenceError: Event is not defined.

Instead we should do:

let loopchange;

// later inside function call:
loopchange = loopchange || new Event('loopchange');

Since this module is only used on the client side we'll then be fine.

We have some new pieces of partially or fully audio-independent state: shuffle and repeatStrategy. There should be corresponding prop callbacks: onShuffleUpdate and onRepeatStrategyUpdate.

onShuffleUpdate will receive the boolean value of shuffle, and it is simple enough to call - it can be triggered on each toggleShuffle.

onRepeatStrategyUpdate, which will receive the string repeatStrategy value, is more complicated, since it is computed as a result of both cycle and the audio element's loop attribute - which is able to be modified externally. This callback should therefore be fired whenever loop is updated, and we should ensure that corresponding updates to the cycle state will happen before the loop listener is activated. This way, we won't alert the parent with incorrect repeatStrategy values.

Also, we should update the onActiveTrackUpdate callback to only be passed the current track index, not the playlist, which should already be known to the parent. This will make it consistent with the other callbacks we are adding.

I love the player and I tried a variation of it but It seems yours is missing the basic volume bar

The isSeekUnavailable method checks this.audio. It should rely on a piece of state, to ensure it's called (and passed to control components) whenever its computed value changes.

The contribution guidelines recommend testing by creating a symbolic link from your local build directory to your node_modules directory to test your build in your project (via ln -s). This is kind of weird, and the instructions don't apply to Windows systems.

Instead, I think we should probably recommend installing the package via local path.

Before committing, we should ensure this routine works without errors. Other methods (e.g. npm link) have been known to cause problems.

The controls prop accepts an array containing strings and objects matching PropTypes.element, though we're actually looking for React classes, not elements. Since a React component class can alternatively be defined as just a render function accepting props, it might be best to test for PropTypes.func and call it a day.

There should be a shuffle boolean value stored in state, with a default which can be set by defaultShuffle. We should also implement a corresponding control toggle.

If shuffle is true, then new tracks will be chosen not in sequential order, but by Math.floor(Math.random() * playlist.length), and cycle will be ignored.

Note that shuffle will be ignored if loop is true, but no extra code is necessary, since the ended event will never be fired for the playing track.

In order to support easier visual configuration, Sass values that would commonly be overridden should be stored at the top of index.scss as variables. These values would include (but not be limited to) "magic" numbers and values which are particularly opinionated.

Please see react-gif-player's GifPlayer.scss for a good example of what this can look like.

Each variable should use the !default flag so that it can be preempted if a user has already defined it in another Sass sheet. E.g.

$audio_player_bg_color: #333 !default;

There's not strictly a right or wrong way to do this.

Variables should be snake_cased and begin with $audio_player_.

There's no visual indication that anything has happened when you hover above and select button controls. This should be remedied.

Since it takes the props style, className and progressClassName, a progressStyle prop would make enough sense. It can be merged with the result of getProgressStyle(progress, progressDirection).

If we use css modules for style loading, class names will be made unique automatically and there will be no need to namespace/nest everything under .audio_player (nesting makes style computation slower).

In addition to audioplayer.css we should build individual css files for each scss module so users can selectively include styles in their projects.

We should cite the sources of icons used in some of our new controls, including volume (icono), as well as shuffle and repeat (google material design icons). It's also worth citing the codepen source of our play/pause button styles (currently commented in stylesheet).

It seems contrary to the philosophy of a responsive, reusable component for it's positioning on the page to be so hard coded in its default stylesheet.

As a short term hack in my project, I've just done this (I'm using CSS-Loader with webpack):
AudioPlayer.jsx

import React from 'react';
import ReactAudioPlayer from 'react-responsive-audio-player';
// Import default CSS
import 'react-responsive-audio-player/dist/audioplayer.css';
// Override default CSS with my own styles
import './AudioPlayer.css';

const AudioPlayer = (props) => {
  return <ReactAudioPlayer {...props} />
};

export default AudioPlayer;

AudioPlayer.css

.audio_player {
  position: relative;
}

where I made the component more generically responsive and positioned relative inside its parent container.

I don't have a proposed solution right at this very moment, but thought I would throw this out there as something to consider.

The HTMLAudioElement supports a srcObject property which can be a Media Stream. This library will be more flexible and useful for modern applications if it can support this API. That will require both research and implementation time. It's unclear whether it will make more sense to integrate media stream support into the existing player, or if there should be a separate player implementation which supports media streams. Ideally, a single playlist could be made up of both string sources and MediaStream objects.

Use of hideBackSkip, hideForwardSkip or disableSeek should trigger deprecation warnings in the JavaScript console. Users should be able to begin using the controls prop instead, which should be specified as an array of string keys matching controls to display. The controls prop implementation will be incomplete compared to the 2.0 release, as it will not be possible to pass in custom components, but it will be more than possible to cover the prior functionality offered by ^1.1.0.

As a replacement for disableSeek users will be able to specify "progressdisplay" (rather than "progress") to render an audio progress pane without event listeners (similar to the AudioProgressDisplay control being added for 2.0 [#58]).

Note that the key assignment algorithm discussed in #57 should be applied here.

This has been a really useful module, thank you for that. With recent updates to react, PropTypes will soon be deprecated in favor of a separate npm package, prop-types.

Could you update the current build to use prop-types?

Here's a quick explanation of what's new in React...soon to be React 16.0.0
https://facebook.github.io/react/blog/2017/04/07/react-v15.5.0.html#migrating-from-react.proptypes

Oops! Too big...

The readme is starting to get somewhat long and confusing, and will get more confusing with the next major release. All the documentation should be moved to a new website, which will replace the existing GitHub Pages deployment.

Since default fixed positioning has been removed the placeAtTop prop to the component should also be removed. This entails:

• Remove placeAtTop from AudioPlayer.propTypes
• Remove relevant annotation from above AudioPlayer class definition
• Remove conditional application of top class to .audio_player element
• Remove .audio_player.top rules from index.scss
• Remove any mention of placeAtTop from README.md

Since loop will be part of state (whose default will be set by the defaultLoop prop #40) it would make sense for the cycle option to also be stored in state, with a corresponding defaultCycle prop. This would allow control implementations to control the status of cycle.

Except: Since loop being true takes strict precedence over cycle, there's no real point of allowing the user to control these properties independently. While the audio loopchange event -> loop state change listener should remain, the only exposed API around cycle or loop should be a new piece of state called repeatStrategy with a corresponding prop defaultRepeatStrategy and a callback passed to controls, onSetRepeatStrategy.

repeatStrategy is an enumerable which can be: "playlist" (loop: false, cycle: true), "track" (loop: true) or "none" (loop: false, cycle: false).

Along with this, we should provide a RepeatButton control which can be clicked to loop between the three repeatStategy options (none, playlist, track).

Currently, dragging outside of the audio progress bar means that the target timestamp stays the same as it was when the drag last occurred within the progress bar div. Instead, the timestamp should update with the drag's x position, regardless of its y position on the page.

If you call .pause on the audio element the paused state does not get set to true

It's not that interesting, compared to more composable components like ProgressBar. One less thing where we would have to worry about outside consumers.

Not a real deprecation since nothing's been released.

Really the only reason to have the disableSeek property was because control options were so locked in for 1.x. With 2.x it is trivial to create an AudioProgressDisplay component which uses ProgressBarDisplay directly rather than the interactive ProgressBar wrapper. If users want a read-only audio progress display, they should use this.

However if for some reason the disableSeek prop is needed by a custom component, it can just be passed to AudioPlayer as before, and since AudioPlayer will no longer recognize that prop, it will be automatically passed down to the controls, which can use it as desired.

Currently, we're just using array indices as keys for our rendered list of control components. But if the contents of our controls list changes on a component update, we should be able to determine which newly-listed controls match those which were previously mounted. This will be particularly important if a control instance is holding its own state, which we would prefer not to lose.

Now that sass overridability has been merged into the latest npm release of icono (1.3.1) we don't need to use a custom fork anymore.

JavaScript linting should be part of the development process.

The provided skip buttons use the "rewind/fastforward" symbol instead of the "skip" symbol (which includes a vertical bar next to the pointed end). For the upcoming major release this should be fixed.

Rapid-fire skips can lead to a console error: Uncaught (in promise) DOMException: The play() request was interrupted by a call to pause().

While this error doesn't (yet) appear to have any particular negative consequences, we should try to avoid throwing it.

This answer from Stack Overflow may help us think about how to solve the problem.

The namespace rr_audio_player is excessively long, especially for class names which add up on the bundle size. We should switch to rrap.

I've noticed that occasionally, after the computer has been suspended for a bit, playback fails on resume and we get the same error message from #28.

This is hard to reproduce. On my MacBook Pro (mid-2015) I've been unable to trigger it intentionally, even when configuring standby mode to commence immediately after sleep.

Obvious workaround: refresh the page.

What about change current audio track through props?

I am using https://themeteorchef.com/base/ as the starting point. I replaced Bootstrap with material.

The player works fine until you navigate at which point it resets. I'm not entirely sure what is conflicting but my best guess thus far is either react route or rc-menu. It seems the state of the player is updated each time you navigate. This can be seen in react dev tools. As I navigate via the menu you can see the activeTrackIndex being updated. The weird part is I have the player as a top level component. I am going to setup some test latter to try and narrow it down to either react router or rc-menu.

If you have any suggestion please let me know. Your player fits my requirements perfectly.

Here is the main App class so you can see how the player is used.

  import React from "react";
  import AppNavigation from "../containers/app-navigation";
  import { Footer } from "../components/footer/footer.jsx";
  //import { Player } from "../components/player/player";
  import AudioPlayer from "react-responsive-audio-player";
  import Container from "muicss/lib/react/container";

  var playlist =
  [{ url: "shows/testshow9/mpthreetest.mp3",
    displayText: "Track 3 by Some Artist" },
    { url: "shows/testshow9/mpthreetest.mp3",
      displayText: "Some Other Artist - Track 2" }];

export class App extends React.Component {
  constructor(params) {
    super(params);

    // this.playlist =
    // [{ url: 'shows/testshow9/mpthreetest.mp3',
    //   displayText: 'Track 3 by Some Artist' },
    //   { url: 'shows/testshow9/mpthreetest.mp3',
    //     displayText: 'Some Other Artist - Track 2' }];

  }

  render() {
    return <div>
      <AppNavigation />
      <Container>
        { this.props.children }
      </Container>
      <Footer/>
      <AudioPlayer playlist={ playlist } />
    </div>;
  }
}

When the cycle prop to AudioPlayer is false:

• It should not be possible to skip before the first track (to the last track) or past the last track (to the first track)
• In the default UI, if back or forward skipping is unavailable, the disabled controls should "look disabled"; i.e. greyed out, default cursor instead of pointer

In all cases:

• A pair of new boolean control component props, backSkipUnavailable and forwardSkipUnavailable should be passed down from AudioPlayer

ETA: skipping UI portion of this since it's unclear how to treat the back skip button. In all cases back skipping is permitted, but sometimes it only goes to time=0 on the current track.

Work will be precipitated by conclusion of #23.

As discussed in #1, it would be difficult to provide a single volume control component to satisfy the needs of each user.

In order to make this library more flexible and powerful for the average developer, we should make it possible to substitute the default audio controls with custom implementations. In order to do this, we should break the existing controls into new React component modules.

If a user wishes to override the default components, they can provide a list of controls in the order they prefer. Each item in the list can be either a string keyword matching one of the default controls, or a React component which will use props passed by the AudioPlayer parent.

Each control component (supplied by the library or the user) will receive the same set of props - that set should be determined.

We could use some component tests, as the API has become a bit complex to validate changes just by loading example.html in a web browser.

Because we're using webpack 1, we have to use module.exports = AudioPlayer rather than export default AudioPlayer; if we didn't, the library would not properly attach as window.AudioPlayer when included as a <script>, but as window.AudioPlayer.default.

Problem: we can't properly do regular and default exports in this way. For example, we could do:

module.exports = AudioPlayer;
module.exports.PlayPauseButton = PlayPauseButton;

// in another file
import AudioPlayer from 'react-responsive-audio-player';
console.log(AudioPlayer.PlayPauseButton);

but it wouldn't be reasonable to expect we could also do: (well apparently this works for now in Babel, so whatever!!)

import { PlayPauseButton } from 'react-responsive-audio-player';
console.log(PlayPauseButton); // undefined!

And if we did...

export { PlayPauseButton };
module.exports = AudioPlayer; // bye PlayPauseButton!

... then we would have another problem - module.exports gets overwritten after PlayPauseButton is added!

Webpack 2 fixes this by letting us use export default and preserve UMD integrity. And now that we want to handle default and regular exports from our root module, the upgrade is necessary.

A number of config options should be available to the user:

Read once, at mount time:

• defaultVolume (double between 0 and 1; default 1)
• defaultMuted (boolean; default false)
• defaultLoop (boolean; default false)
• defaultPlaybackRate (number; default 1)
• startingTime (number; default 0)
• startingTrackIndex (number; default 0)

Updated whenever received:

• crossOrigin (enum['anonymous', 'use-credentials']; no default)

In order to continue simplifying the release process as discussed in #43, we should host both unminified and minified distributables on npm - allowing us to link to all our releases via unpkg, and cut out the need to maintain a separate dist branch and upload releases to GitHub.

The easiest way to facilitate this is to make npm run build create additional minified versions of the js and css assets created by webpack.

While most state internal to the AudioPlayer component is accessible via audio listeners added to the onMediaEvent hash, one important piece of information, the active track, is not. We should make it possible to pass a prop called onActiveTrackUpdate which receives the new track index and the loaded playlist as arguments whenever the active track changes.

Among other uses, this will make it possible to externally track and serialize AudioPlayer state information for restoration at a later point (via props like startingTrackIndex, defaultMuted, etc.).

While testing the solution for #57 I found that when control layout is shuffled without a page resize, components such as ProgressBar which rely on knowledge of their position within the screen begin to fail. As far as my ProgressBar instance knows, its bounding box is still on the right side of the screen, even though it has since shifted to the left.

One solution could be to pass a layoutString which could be a hash of the concatenated stringified forms of the components being rendered. A couple cons:

• Obviously no guarantee of no hash collision
• There's another issue.......

We have a deeper problem in the scenario where a component adjacent to our bounding box-aware component grew or shrank, causing our bounding box to move, but not resize. There was no change in component order, no change in bounding box size, but it's no longer in the same place.

Essentially, we need a way to not only observe element resizing, but repositioning as well.

One "failsafe" but almost certainly not performant method would be to compute the bounding box each time it's required.

This solution is needed for 2.0, but it will also need to be in the 1.2 release after the algorithm solution to #57 is applied.

Releasing this package typically involves:

1. Get code to releasable state.
2. npm run build
3. `cp dist/* ../another-instance-of-this-repo-on-the-dist-branch/
4. cd ../another-instance-of-this-repo-on-the-dist-branch/ && npm run build (to minify the css and js files)
5. git add . && git commit -m "v[x].[x].[x]" && git push origin dist
6. Log onto GitHub and:
   a. Navigate to the dist branch.
   b. Click on the latest commit to get the repo snapshot (skipping this is bad!).
   c. Navigate to audioplayer.js and select "raw".
   d. Switch "raw.githubusercontent" in the url to "cdn.rawgit". Copy the url and save it.
   e. Change ".js" at the end of the url to ".css", press enter and save this url too.
7. cd ../react-responsive-audio-player/
8. Open README.md and replace the script urls with the new ones we copied, and their version comments.
9. Open package.json and bump the version number.
10. git add . && git commit -m "[x].[x].[x] version bump, CDN script sources in readme replaced." && git push origin master
11. npm publish
12. Log onto GitHub and:
    a. Prepare release notes for new version.
    b. Drag in contents of dist branch to attach.
    c. Submit.

But with one magical step, replacing the scripts in our readme with unpkg scripts (e.g. https://unpkg.com/react-responsive-audio-player/dist/audioplayer.js) we can eliminate having to update those scripts ever again - because it pulls straight from npm!

This will cut out steps 6-8. Or 3-8, if we decide to stop uploading copies of the built library to GitHub altogether. If we include all the sub-bullets, that shortens this process from 20 to 9 steps... still several steps, but fewer, and I get to stay in the terminal until the package is published. Sounds like a win to me!

If the progress bar container resizes independently of the window, the boundingRect won't be re-computed appropriately. Instead perhaps we should use css-element-queries to listen for resize events.

I am trying to play() the audio from a wrapping component.

I am including the AudioPlayer like this

<AudioPlayer playlist={playlist} hideBackSkip={true} ref="audioPlayer" />

Then in the wrapping component I try to play the audio inside componentDidMount()

componentDidMount() {
  this.refs.audioPlayer.audio.play();
}

The audio is only playing automatically after the first user interaction (i.e. button click) – Do you know why?

I saw the "autoplay" prop and want to start play() programmatically to make it work in mobile safari.

#33 introduced a new bug where dragging the mouse too far to the left on drag-to-seek will attempt to set the currentTime to negative (no safety check on input).

Only reproduced on the master branch - the next branch (development for 2.0 release) does not have this issue.