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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] New playlist functionality #233

Closed
wants to merge 22 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@vgezer
Contributor

vgezer commented May 10, 2014

馃毀 Work in progress!

Ready:

  • Sidebar with playlists
  • Hardcoded playlists
  • Hardcoded song display
  • Playlist - Database connection
  • Create basic playlist functionalities (create, edit, delete playlist)
  • Play the next song in the playlist after the current song
  • Add songs by drag&drop
  • Remove songs from playlists

Working on:

  • Design improvements

To do:

  • Unit tests
  • Documentation

Playlists will:

  • Repeat playlist songs

Playlists may:

  • be sorted by dragging

... and will be updated

Show outdated Hide outdated js/config/app.js
Show outdated Hide outdated js/config/app.js
Show outdated Hide outdated templates/main.php
@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke May 28, 2014

Member

@wakeup There was an issue with the route handling. I fixed it and also cherry-picked the fix to master (06ba6d3) Give it a try.

Member

MorrisJobke commented May 28, 2014

@wakeup There was an issue with the route handling. I fixed it and also cherry-picked the fix to master (06ba6d3) Give it a try.

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer May 28, 2014

Contributor

Thanks. This was also another question that I forgot to ask :). Thanks

Contributor

vgezer commented May 28, 2014

Thanks. This was also another question that I forgot to ask :). Thanks

@vgezer vgezer self-assigned this May 28, 2014

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke May 31, 2014

Member

@wakeup Maybe give a rebase and squashing session a try :) If you have worries: just start a new branch from this one to have a pointer to this state and be able to easily recover from any mistakes during the git learning phase, but there is nothing better than learning by doing and better learn to rebase and squash early ;) If you need help. I'm available in the complete next week between 10 and 20 for ownCloud \o/ (and of course can you help with any problems that arise)

Member

MorrisJobke commented May 31, 2014

@wakeup Maybe give a rebase and squashing session a try :) If you have worries: just start a new branch from this one to have a pointer to this state and be able to easily recover from any mistakes during the git learning phase, but there is nothing better than learning by doing and better learn to rebase and squash early ;) If you need help. I'm available in the complete next week between 10 and 20 for ownCloud \o/ (and of course can you help with any problems that arise)

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke May 31, 2014

Member

@wakeup Awesome :) You mastered it 馃懐

Member

MorrisJobke commented May 31, 2014

@wakeup Awesome :) You mastered it 馃懐

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer May 31, 2014

Contributor

@MorrisJobke finally I managed to rebase and squash, but what happened to routes_api.php :)

Contributor

vgezer commented May 31, 2014

@MorrisJobke finally I managed to rebase and squash, but what happened to routes_api.php :)

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke May 31, 2014

Member

Could it be that you rebase ontop of a outdated master?

Member

MorrisJobke commented May 31, 2014

Could it be that you rebase ontop of a outdated master?

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer May 31, 2014

Contributor

But when I was working before, we had that file lying here: https://github.com/owncloud/music/tree/master/appinfo

Now in master, it is disappeared :s

Contributor

vgezer commented May 31, 2014

But when I was working before, we had that file lying here: https://github.com/owncloud/music/tree/master/appinfo

Now in master, it is disappeared :s

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke May 31, 2014

Member

@wakeup Ah ...now I remind ... I moved it to the new interface of the new core API

Member

MorrisJobke commented May 31, 2014

@wakeup Ah ...now I remind ... I moved it to the new interface of the new core API

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer May 31, 2014

Contributor

@MorrisJobke I've found it, but now I cannot access to the getUser() method [here](3e526c6#diff-b9e15819672f6817a033ecc447a6e2a2R115

Nevermind, found it :) Thanks

Contributor

vgezer commented May 31, 2014

@MorrisJobke I've found it, but now I cannot access to the getUser() method [here](3e526c6#diff-b9e15819672f6817a033ecc447a6e2a2R115

Nevermind, found it :) Thanks

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke May 31, 2014

Member

@wakeup Weird... the naming is correct.

Member

MorrisJobke commented May 31, 2014

@wakeup Weird... the naming is correct.

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer May 31, 2014

Contributor

Managed to get it. Now we can get the value like a member variable instead. Not sure why it has changed.

Contributor

vgezer commented May 31, 2014

Managed to get it. Now we can get the value like a member variable instead. Not sure why it has changed.

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Jun 9, 2014

Member

New playlist should look like this:
out

Member

MorrisJobke commented Jun 9, 2014

New playlist should look like this:
out

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Jun 9, 2014

Member

Rename should look like this:
rename-calender

Have a look at the calendar source code to integrate this.

This would be super awesome \o/

Member

MorrisJobke commented Jun 9, 2014

Rename should look like this:
rename-calender

Have a look at the calendar source code to integrate this.

This would be super awesome \o/

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Jun 9, 2014

Member

Adding songs to a playlist should work by drag and drop (draging artist, album and track should work) on the playlist name in the sidebar

Member

MorrisJobke commented Jun 9, 2014

Adding songs to a playlist should work by drag and drop (draging artist, album and track should work) on the playlist name in the sidebar

Show outdated Hide outdated ajax/plist-new.php
@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Aug 6, 2014

Member

I updated the JS libraries in master and rebased (then this PR will get a lot lighter removes 9.200 deletions from this PR)

Member

MorrisJobke commented Aug 6, 2014

I updated the JS libraries in master and rebased (then this PR will get a lot lighter removes 9.200 deletions from this PR)

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Aug 6, 2014

The inspection completed: 6 new issues, 2 updated code elements

scrutinizer-notifier commented Aug 6, 2014

The inspection completed: 6 new issues, 2 updated code elements

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Aug 6, 2014

Member

@wakeup The whole styling of sidebars has changed in core. So I currently try to use as much from core. Then this will also benefit from the mobile responsiveness of the core. Are you okay with that?

Member

MorrisJobke commented Aug 6, 2014

@wakeup The whole styling of sidebars has changed in core. So I currently try to use as much from core. Then this will also benefit from the mobile responsiveness of the core. Are you okay with that?

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Aug 6, 2014

Member

@wakeup Design-wise we need a complete overhaul to use all this fancy stuff :(

Member

MorrisJobke commented Aug 6, 2014

@wakeup Design-wise we need a complete overhaul to use all this fancy stuff :(

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Aug 6, 2014

Contributor

@MorrisJobke Yes, but the worst problem is the dissappearing of the sidebar for mobile views. I am thinking of adding an arrow for mobile views to add into playlist. What do you think?

Contributor

vgezer commented Aug 6, 2014

@MorrisJobke Yes, but the worst problem is the dissappearing of the sidebar for mobile views. I am thinking of adding an arrow for mobile views to add into playlist. What do you think?

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Aug 6, 2014

Contributor

@MorrisJobke Do you mean the CSS stuff or the whole programming approach?

Contributor

vgezer commented Aug 6, 2014

@MorrisJobke Do you mean the CSS stuff or the whole programming approach?

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Aug 6, 2014

Member

@wakeup Just the CSS part.

Member

MorrisJobke commented Aug 6, 2014

@wakeup Just the CSS part.

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Aug 6, 2014

Member

@wakeup We can design quite a different interface (maybe add a menu button) to add to existing playlists just for mobile. But getting it into a working state for desktop user is the goal of this PR I think

Member

MorrisJobke commented Aug 6, 2014

@wakeup We can design quite a different interface (maybe add a menu button) to add to existing playlists just for mobile. But getting it into a working state for desktop user is the goal of this PR I think

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Aug 6, 2014

Contributor

@MorrisJobke Currently, the mobile users will be able to replay all songs in their playlists, but they will be read-only. For the mobile I think I should open another PR as you said. And I think we can do it after the final evaluation?

Contributor

vgezer commented Aug 6, 2014

@MorrisJobke Currently, the mobile users will be able to replay all songs in their playlists, but they will be read-only. For the mobile I think I should open another PR as you said. And I think we can do it after the final evaluation?

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Aug 10, 2014

Contributor

We should prepare some unit tests for drag&drop, but I am not sure if we can manage those, though. Any help would be appreciated ;).

Also, I need to prepare all code written here as a patch to submit to google :).

Contributor

vgezer commented Aug 10, 2014

We should prepare some unit tests for drag&drop, but I am not sure if we can manage those, though. Any help would be appreciated ;).

Also, I need to prepare all code written here as a patch to submit to google :).

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Aug 11, 2014

Contributor

Continuing on #266

Contributor

vgezer commented Aug 11, 2014

Continuing on #266

@vgezer vgezer closed this Aug 11, 2014

@MorrisJobke MorrisJobke deleted the new-playlist branch Aug 12, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment