benwiley4000/cassette

Consider removing default 'position: fixed', bottom: 0' styles

andrewzey opened this issue · 4 comments

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.

Interesting thought. I don't totally disagree with you.

I included those styles as default for a couple of reasons:

  1. That's what I used before I forked this into a module (not a good reason, but a reason!)
  2. While not at all universal, fixed position at bottom of page is a pretty standard style for web-based audio navigation bars.

Further, my thoughts around styles were that the module should provide a set of reasonable default styles (which the developer can take or leave), and updates should be done via override rather than replacement. Granted, this results in a bit of redundant CSS, but I don't see a much better option (in the general case). I'd like the module to be widely usable out of the box (without requiring the developer to modify any CSS).

That said, removing two CSS rules in order to make the component more portable doesn't sound that bad to me. If we do this I'd say we should also do away with the placeAtTop prop which assumes the component was fixed to the bottom to begin with. If you'd like to submit a PR, go ahead.

@andrewzey are you planning to submit a PR for this issue? I'm trying to clear the issue stack ahead of a 1.0 release, so if you don't plan to handle this one, I'll be happy to.

In order to hasten the 1.0 release I went ahead and corrected this issue. Thanks again for flagging it. See #11. You may now also specify outer element styles via a style prop to AudioPlayer.

Thanks!