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

Room history #337

Merged
merged 3 commits into from Sep 13, 2020
Merged

Room history #337

merged 3 commits into from Sep 13, 2020

Conversation

Gbd199
Copy link
Contributor

@Gbd199 Gbd199 commented Jul 17, 2020

No description provided.

Gbd199 added 2 commits Jul 17, 2020
* new 'roomhistory' value in configuration
* change gui room input to combobox to display history
* update history upon startup using the configuration room
@Et0h
Copy link
Contributor

Et0h commented Aug 9, 2020

Hi,

I have long wanted for a feature such as this so thanks for helping to move it forwards.

Firstly, I've tested and it works. The behaviour is that it lists the rooms you connect to the server with and then provides them as options from the GuiConfig window for future connections, irrespective of server. As far as it goes, this is a reasonable behaviour because trying to make it per-server arguably adds unnecessary complexity when people might use the same room names across multiple servers

Suggestions for consideration/discussion:

  • The ability to delete a room from the list of rooms.
  • Allow users to select from the list of rooms once connected, e.g. by making the textbox in the main Syncplay window next to 'Join room' also be a combo box.
  • Remember rooms you connect to from the main Syncplay window
  • When you create a managed room, also remember the version with the auto-identification in it, e.g. +RoomName:868A81271506:NX-914-891
  • An alternative approach to auto-remembering the room names would be to have a 'favourites' system where, once connected, you can add a room to your list of favourites and then manage that list. This will prevent the list from getting clogged up so requires less user action to clear it, but on the other hand does require users to manually click to remember a room which they might forget to do. Which is more convenient and intuitive?

@Gbd199
Copy link
Contributor Author

Gbd199 commented Aug 13, 2020

thanks for the feedback, I'll see what I can do over the weekend

* add rooms dialog
* new button to open rooms dialog
@Gbd199
Copy link
Contributor Author

Gbd199 commented Aug 14, 2020

I decided to go with a list view dialog for the rooms. This gives the user complete and simple control over the rooms and did not require too many changes. Due to the simple control I think adding a 'favorite rooms' mechanism is pointless, but tell me what you think.

@Et0h
Copy link
Contributor

Et0h commented Aug 14, 2020

Allowing people to edit, add and delete rooms via a multiline textbox dialog makes sense, not least because it is in line with what we do for trusted domains and the playlist. I'll try out your latest version and give it some consideration. Something I'll be thinking about is whether there needs to be an easy way to enable/disable the feature automatically remembering room names.

@Et0h
Copy link
Contributor

Et0h commented Aug 20, 2020

Thought about it, and I think the only thing it really needs is to be able to turn off auto-remembering. If nobody has any further comments/objections then when I have time I'll merge the changes and add any final tweaks myself.

@Gbd199
Copy link
Contributor Author

Gbd199 commented Aug 21, 2020

Allow users to select from the list of rooms once connected, e.g. by making the textbox in the main Syncplay window next to 'Join room' also be a combo box.

Remember rooms you connect to from the main Syncplay window

I looked into it and found some issues with that.
First of all, purely changing the Input into a combobox is fairly simple.
But when it comes to remembering the rooms I encountered a problem, in the current version, if in the main window you join a new room, it will not save it in the configuration for you to see as the default room next time you start syncplay.
Remembering rooms from the main window will require fixing that issue first, which is not hard but does require some changes and proof testing since I'm not familiar with the more complicated functionality of syncplay and might easily miss and mess with some feature.
And it will also require setting room/roomhistory configuration logic in client.py(in a method similar in its idea to setTrustedDomains()) so gui.py can access it.

So in my opinion, as for this pull request, changing the input into a combobox should be enough. Remembering the history form the main window, and fixing the issue I pointed out should be addressed in a new issue.
But tell me what you think

@Et0h Et0h merged commit 86bd299 into Syncplay:master Sep 13, 2020
2 checks passed
@Et0h
Copy link
Contributor

Et0h commented Sep 13, 2020

Further discussion take take place on #336

albertosottile pushed a commit to albertosottile/syncplay that referenced this pull request Sep 30, 2020
* Add room history mechanism (Syncplay#336)

* new 'roomhistory' value in configuration
* change gui room input to combobox to display history
* update history upon startup using the configuration room

* Prevent room history from saving empty room name

* Add rooms editing ability
* add rooms dialog
* new button to open rooms dialog
albertosottile pushed a commit to albertosottile/syncplay that referenced this pull request Sep 30, 2020
* Add room history mechanism (Syncplay#336)

* new 'roomhistory' value in configuration
* change gui room input to combobox to display history
* update history upon startup using the configuration room

* Prevent room history from saving empty room name

* Add rooms editing ability
* add rooms dialog
* new button to open rooms dialog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants