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

A few adjustments to file/dir open dialogs #13252

Merged
merged 1 commit into from
Nov 26, 2017

Conversation

Krakean
Copy link
Contributor

@Krakean Krakean commented Nov 24, 2017

a) Added Backspace key support for Tree-based file dialog.
b) Fixed issue inability to select a folder in project manager (always previous folder was selected instead). Video: https://i.imgur.com/iQe8XAN.gifv (#13242, #13203)
c) Open Directory mode: changed "Open" to "Select Current Folder"
d) Block "Open" button when inappropriate content is selected (for example, file when in open folder mode, or folder when in open files mode)
e) List-based file dialog now support item deselecting (when you click on empty space). Video: https://i.imgur.com/i90oId9.gifv

P.S. Require PR #13245 to function/compile - for d) & e)

Fixes #13203.

a) Added Backspace key support for Tree-based file dialog.
b) Fixed issue inability to select a folder in project manager (always previous folder was selected instead).
c) Open Directory mode: changed "Open" to "Select Current Folder"
d) Block "Open" button when inappropriate content is selected (for example, file when in open folder mode, or folder when in open files mode)
@akien-mga
Copy link
Member

Overall this is good, but I see a couple issues that should be addressed IMO:

  • The new "Open" button text "Select Current Folder" does not make sense when you have selected a subfolder and want to pick this one (there are two workflows: either you browse in the final folder and expect it to be the "current" one that will be picked if nothing is selected, or you browse to its parent and select it in the ItemList before pressing Return/Open). So the text should maybe be set dynamically based on the context (item_list->is_anything_selected(), as done already for the files mode).
  • It's confusing that .. is selected by default but ignored, and that pressing "Select Current Folder" with .. selected opens the current folder. I agree that opening the parent folder is even worse, but that's still bad UX. My proposal:
    • Remove .. just like . was removed already.
    • Add a "Parent folder" button to replace it
    • When entering a folder, nothing should be autoselected (i.e. it should default to "Select Current Folder" by lack of selection). The cursor key Down should then select the first entry of the ItemList.

If you agree with this proposal and can implement it quickly, that would be best, as this is the main blocker for 3.0 beta IMO.

@akien-mga
Copy link
Member

Actually I'll merge this version once the CI is happy, since it fixes an annoying bug. I'd still be interested in a follow-up PR with the enhancements I mentioned above. :)

@Krakean
Copy link
Contributor Author

Krakean commented Nov 26, 2017

@akien-mga Completely agree with proposal, will implement updated(or new one if this one will be merged already, which is even preferably I suppose) in Monday before 14pm Europe time.

@akien-mga akien-mga merged commit 2755eeb into godotengine:master Nov 26, 2017
@Krakean Krakean deleted the file_dialogs_small_tweaks branch November 27, 2017 07:01
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.

2 participants