Record Audio UI - Initial #24012
Record Audio UI - Initial #24012
Conversation
| @@ -146,6 +153,30 @@ export default class AssetManager extends React.Component { | |||
| </span> | |||
| </div>); | |||
|
|
|||
| const recordButton = (<div> | |||
There was a problem hiding this comment.
Style nit: I know this file hasn't done this so far, but I prefer stateless functional components and composition as an organizing principle.
// Defined in the file's top scope:
const RecordButton = () => (
<div>
<button {...}>
<i {...}/>
Record Audio
</button>
</div>
);
const Buttons = ({recordAudioEnabled}) => (
<div>
{recordAudioEnabled && <AudioRecorder {...}/>}
<span style={...}>
{uploadButton}
{recordAudioEnabled && <RecordButton/>}
</span>
</div>
);
Buttons.propTypes = {
recordAudioEnabled: PropTypes.bool,
};
{experiments.isEnabled('recordAudio') && <RecordButton/>}
// in render()
<Buttons recordAudioEnabled={experiments.isEnabled('recordAudio')}/>There was a problem hiding this comment.
I also prefer this style and it reads more 'React-y', in my opinion. I added a stateless functional component for RecordButton. However, with a change I made to the when AudioRecorder gets displayed, the buttons is now tied into the state of the AssetManager (i.e. has a button been selected). In this case, I think it is no longer advantageous to move buttons to its own component, but I'm definitely able to be convinced that it's still preferable. I'd love to hear your opinions!
| className="share" | ||
| > | ||
| <i className="fa fa-microphone" /> | ||
| Record Audio |
There was a problem hiding this comment.
Might as well i18n now.
| const buttons = ( | ||
| <div> | ||
| {experiments.isEnabled('recordAudio') && | ||
| <AudioRecorder isVisible={this.state.recordingAudio}/> |
There was a problem hiding this comment.
Since the state for showing/not showing the recorder component lives up here, why not put the logic for it here as well?
{experiments.isEnabled('recordAudio') &&
this.state.recordingAudio &&
<AudioRecorder/>}|
|
||
| render() { | ||
| if (this.props.visible) { | ||
| return <div></div>; |
There was a problem hiding this comment.
Nit: Unless you need the empty div for some reason, fine to return null to render nothing. This might also be moot, if logic for visibility of this component moves into the parent.
| <button | ||
| onClick={this.onStopClick} | ||
| id="stop-record" | ||
| style={{...styles.blueButton, ...styles.button}} |
| <input type="text" placeholder="mysound1.mp3" onChange={this.onNameChange} value={this.state.audioName}></input> | ||
| <span> | ||
| <button | ||
| onClick={this.onStopClick} |
There was a problem hiding this comment.
this.onStopClick is not defined.
| @no_mobile | ||
| Feature: Manage Assets | ||
|
|
||
| # This scenario will be removed when the feature is released. |
| @@ -1486,3 +1486,10 @@ def get_section_id_from_table(row_index) | |||
| @browser.execute_script(code) | |||
| wait_short_until {@browser.execute_script('return window.signOutComplete;')} | |||
| end | |||
|
|
|||
| Then /^I open the Manage Assets dialog$/ do | |||
There was a problem hiding this comment.
👍 Yay steps that read as English!
|
@islemaster PTAL, thanks! |
This is the basic UI for the Record Audio feature. Includes initial buttons that I can break out before incorporating the audio recording library. Behind the experiment flag 'recordAudio'.
Before:
After:
