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.

Sign up for GitHub

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 audio track number not displayed and adds audio disk (media) number #1454

Merged
merged 6 commits into from
Sep 25, 2018
Merged

Fix audio track number not displayed and adds audio disk (media) number #1454

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a30d804
Fix issue track number not displayed if total number of tracks is not…
Borewit Aug 15, 2018
c2594f5
Add disk number in addition to track number.
Borewit Aug 15, 2018
c7f30df
Update order of audio properties from: album, track, disk, format to …
Borewit Aug 15, 2018
e2448ca
Return JSX block.
Borewit Aug 18, 2018
4306569
Get rid of third parameter which is replaced by CSS capitalize
Borewit Aug 19, 2018
8e91840
Fixed error when value is undefined.
codealchemist Aug 19, 2018
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
40 changes: 30 additions & 10 deletions src/renderer/pages/player-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,27 @@ function renderOverlay (state) {
)
}

/**
* Render track or disk number string
* @param key should be either 'track' or 'disk'
* @param label should be either 'Track' or 'Disk'
*/
function renderTrack (common, key, label) {
// Audio metadata: track-number
if (common[key].no) {
let str = `${common[key].no}`
if (common[key].of) {
str += ` of ${common[key].of}`
}
const style = { textTransform: 'capitalize' }
return (
<div className={`audio-${key}`}>
<label style={style}>{key}</label> {str}
</div>
)
}
}

function renderAudioMetadata (state) {
const fileSummary = state.getPlayingFileSummary()
if (!fileSummary.audioInfo) return
Expand All @@ -227,6 +248,15 @@ function renderAudioMetadata (state) {
))
}

// Audio metadata: disk & track-number
const count = ['track', 'disk']
count.forEach(key => {
const nrElem = renderTrack(common, key, key[0].toUpperCase() + key.substring(1))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can use CSS to capitalize the label.
If we do that we would only pass key and avoid operating directly on the string.

On renderTrack we would be able to do:

    const style = { textTransform: 'capitalize' }
    return (
      <div className={`audio-${key}`}>
        <label style={style}>{key}</label> {str}
      </div>
    )

Copy link
Member Author

@Borewit Borewit Aug 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I have no experience with React, I blindly followed your suggestion.
But seems to do the job, so well done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here we can remove the third param, which is not needed anymore.
:)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, yeah sure.

if (nrElem) {
elems.push(nrElem)
}
})

// Audio metadata: album
if (common.album) {
elems.push((
Expand Down Expand Up @@ -269,16 +299,6 @@ function renderAudioMetadata (state) {
))
}

// Audio metadata: track-number
if (common.track && common.track.no && common.track.of) {
const track = common.track.no + ' of ' + common.track.of
elems.push((
<div key='track' className='audio-track'>
<label>Track</label>{track}
</div>
))
}

// Audio metadata: format
const format = []
fileSummary.audioInfo.format = fileSummary.audioInfo.format || ''
Expand Down