Skip to content
This repository has been archived by the owner on May 19, 2022. It is now read-only.

Config File Switching #80

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

Ligraph
Copy link

@Ligraph Ligraph commented Dec 10, 2016

WIP.

Features I want:

  • Can switch config files and thumbnails load properly
  • Program remembers what your last config file was and loads it
  • Create a default config file if none exist in the directory
  • Copy config files from UI
  • Delete config files from UI (need are you sure dialog)
  • Rename config file names from UI (use a dialog for name?)

Also need a better looking UI.

@Phrynohyas
Copy link
Owner

I have to ask you to redo the pull request :(

Iuuse is that you've commited a lot of packages that shouldn't be in the repository. Cleaning them out if I'll merge this branch will be a mess

Which Git client do you use that allows to commit way to much files?

@Ligraph
Copy link
Author

Ligraph commented Dec 11, 2016

I've been using the addon for Visual Studio. I think I forgot to add packages to the gitignore.

@Ligraph
Copy link
Author

Ligraph commented Dec 11, 2016

The extra files are fixed (they shouldn't be part of the pull request any more), do you still need me to create a new pull request?

@Phrynohyas
Copy link
Owner

Not needed.

Actually I'd like to discuss with you the following idea - to make the config selection code and GUI a part of a separate dialog (for now it can be opened via button press in the main dialog)

Thus the code will be separated and the main GUI will be keep clean (actually I think the main gialog is overcomplicated already for an average user, so adding even more controls won't help)

@Ligraph
Copy link
Author

Ligraph commented Dec 12, 2016

I like that idea, I'll see what I can figure out. For now its finals week so I probably won't get much done in the next few days.

What if instead of a whole separate dialog the main window just expanded to the side? I'm not sure how that would work in the code, but it seems like it would be a little cleaner UI wise. And we could put other controls there eventually.

@Phrynohyas
Copy link
Owner

Separate dialog is better because this functionality (config switching) is completely different from the rest of the main dialog options.
I'll eventually move that dialog to the tray menu so it will be completely separated from the main options dialog

@Ligraph
Copy link
Author

Ligraph commented Dec 26, 2016

I created a separate dialog for renaming, moving, copying, and deleting. I left the dropdown in because it is small, and I think having to open and click through a separate dialog just to change configs would be a pain.
UI so far:
image

I still need to add the rightclick tray menu and streamling the config file renaming (remove config/ and .json).

@Ligraph
Copy link
Author

Ligraph commented Dec 26, 2016

Need to test that config files load correctly with deleted/renamed events.

@Ligraph
Copy link
Author

Ligraph commented Dec 26, 2016

So, if you delete your last config, thumbnail positions will not reset. This is not intended, but occurs because in RefreshThumbnails and UpdateThumbnailsList when GetThumbnailLocation is called the current thumbnail position is passed as the default, thus, since the newly generated config is blank, the current positions are used. Note that they are not saved because they are never written to the config file, unless someone moves or resizes them.

As far as I can tell everything else is working properly, I'm going to try to handle this in the next few days, but I'm not yet sure of the best way. Unless you have any objections I plan on just using the default position on startup (whatever/wherever that is).

Edit: This also means that if a thumbnail position is not included in a config it will automatically go to the default position instead of its current position when the config is loaded, which I think is a positive (It mimics the current behavior since without config switching the current position will be the form default position, which is what I will use).

@eskwire
Copy link

eskwire commented Dec 26, 2016

Might be could test this with 4+ accounts for you.

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

Successfully merging this pull request may close these issues.

3 participants