File Dialog UI component - #294
Conversation
|
Hello @Nibba2018! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-06-15 20:26:53 UTC |
Nibba2018
added
🚀 🐍 GSOC 2020
Codecov Report
@@ Coverage Diff @@
## master #294 +/- ##
==========================================
- Coverage 89.03% 88.80% -0.23%
==========================================
Files 23 21 -2
Lines 5419 5183 -236
Branches 702 665 -37
==========================================
- Hits 4825 4603 -222
+ Misses 409 406 -3
+ Partials 185 174 -11
|
|
Hello @skoudoro , can you have a look at this skeletal implementation and let me know what else to add? You can run |
skoudoro
left a comment
skoudoro
left a comment
There was a problem hiding this comment.
Looks good, I like the simplicity @Nibba2018. you can continue in this direction.
- Concerning design, you should emphasize a bit more each element (like you did on your sketch by showing the border of each element).
- We should define at the init the filedialog type (save or open)
- Can you explain why you choose a
TextBlock2Dfor accept and reject button instead of abutton2D?
|
Hello @skoudoro , thank you for the review. Here are some of my questions.
|
skoudoro
left a comment
skoudoro
left a comment
There was a problem hiding this comment.
Hi @Nibba2018,
Thank you for the update @Nibba2018
By showing more emphasis do you mean adding more gap between the elements? I could set the color of the parent panel to be the same as that of the background of FileMenu2D to make it more seamless.
I meant border, but we might need to do that for all widgets, specially panel
I wasn't sure how to display text on top of a button, so I chose TextBlock2D for demonstration purposes first.
2 solutions:
- We need to improve button2D to accept text ( you can make a new PR for this)
- We need to improve text2D to allow a border (@antrikshmisri is working on this)
In any case, this PR will need some other PR before merging it. It also needs to be rebased.
Thank @Nibba2018
antrikshmisri
left a comment
antrikshmisri
left a comment
There was a problem hiding this comment.
Hey @Nibba2018, here's a quick overview of the PR. I'll have to test some things out first to provide a in depth review.
| multiselection: bool, optional | ||
| {True, False} | ||
| Whether multiple values can be selected at once. | ||
| reverse_scrolling: bool |
| dialog_type="Open") | ||
|
|
||
| # Create temporary directory and files | ||
| os.mkdir(os.path.join(os.getcwd(), "testdir")) |
There was a problem hiding this comment.
You could use something like TemporaryDirectory from tempfile to create temporary directory. That way it will automatically delete the temporary folder and files created under the temporary directory. Nothing critical.
| self.dialog_size = size | ||
| self.directory_contents = [] | ||
|
|
||
| self.bg_color = bg_color |
There was a problem hiding this comment.
You could use np.clip to clip the individual color values between (0, 1)
| self.directory_contents = [] | ||
|
|
||
| self.bg_color = bg_color | ||
| self.text_color = text_color |
|
|
||
| self.bg_color = bg_color | ||
| self.text_color = text_color | ||
| self.selected_color = selected_color |
| self.bg_color = bg_color | ||
| self.text_color = text_color | ||
| self.selected_color = selected_color | ||
| self.unselected_color = unselected_color |
| self.text_color = text_color | ||
| self.selected_color = selected_color | ||
| self.unselected_color = unselected_color | ||
| self.scroll_bar_active_color = scroll_bar_active_color |
|
Superseeding with PR #832 |
5 participants
This PR will add a UI component to choose a file from the file system. The overall design will be similar to the following:
