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.

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

Cleanup preferences page #1498

Merged
merged 2 commits into from
Aug 18, 2019
Merged

Cleanup preferences page #1498

merged 2 commits into from
Aug 18, 2019

Conversation

mathiasvr
Copy link
Contributor

  • Moves path logic to PathSelector component
  • Removes displayValue, and always shows path instead.

I think it makes sense to remove displayValue, because it's only used to display the name of the external player, which is inconsistent with how we display other paths.
Also, the player name is already displayed in a message above the path, so we don't lose that information.

@stale
Copy link

stale bot commented Jan 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale
Copy link

stale bot commented Apr 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Apr 9, 2019
@stale stale bot closed this Apr 17, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jul 16, 2019
@mathiasvr
Copy link
Contributor Author

Let's try this again 😏
It's pretty low priority though, so don't waste too much time on it.

@mathiasvr mathiasvr reopened this Aug 4, 2019
@stale stale bot removed the stale label Aug 4, 2019
@mathiasvr mathiasvr force-pushed the cleanup-preferences branch 2 times, most recently from 48bf406 to 7b39c9a Compare August 4, 2019 19:13
@Borewit Borewit requested a review from feross August 4, 2019 19:48
Copy link
Member

@feross feross left a comment

Choose a reason for hiding this comment

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

LGTM

@webtorrent webtorrent unlocked this conversation Aug 16, 2019
@hicom150
Copy link
Contributor

LGTM! 👍

If this PR gets merged, #1652 and #1653 could be closed 😉

@feross feross merged commit 767d31e into master Aug 18, 2019
@feross
Copy link
Member

feross commented Aug 18, 2019

@hicom150 Thanks for letting me know. I'll close those now :)

Thanks, @mathiasvr!

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

Successfully merging this pull request may close these issues.

3 participants