-
Notifications
You must be signed in to change notification settings - Fork 40
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
UI Only Custom Datadir #392
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the README at https://github.com/bitcoin-core/gui-qml/tree/main/src/qml with the additional packages that need to be installed.
b06ce79
to
ed6d17c
Compare
Good call @johnny9! Just pushed a new commit with the updated doc. Now it's pretty much equivalent (give and take) with @jarolrod's #273 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK 3f56460
It works as expected, left some code-related nits.
3f56460
to
649c2a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double-clicking on Custom
opens the storage selection in the background.
single-click works ok
openbackground.webm
649c2a5 you added qml-module-qtquick-dialogs
, the other 3 qtdeclarative5-dev
qtquickcontrols2-5-dev
qml-module-qtquick-controls2
should be added to the instructions as well?
Thanks @MarnixCroes, good catch... wasn't able to reproduce on WSL Ubuntu 22.04... Found a potential workaround by adding a timer, will implemented in a future PR since it's little out of scope for this one.
those are actually found here in the qml/README.md: Line 66 in 649c2a5
and here: Line 73 in 649c2a5
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 649c2a5
Tested static mobile and desktop builds as well as dynamic linux builds and the FileDialog triggers properly with no QML errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK 649c2a5
I've managed to reproduce the issue reported by @MarnixCroes, on Ubuntu 22.04 it takes a 3 or 4 attempts, but eventually, it occurs. Tried to fix it using visible: true
/ false
but couldn't solve it, it would require more investigation and could be done on a follow-up.
nits (not blockers):
- Since this is UI only changes, I'd leave commit ffd5201 out, it doesn't change anything in terms of user experience and if we find an issue there, we'd need to revert it (or fix it on top). So in
StorageLocations.qml
I'd comment the lines making those calls to theoptionsModel
adding another comment on top of each like "// TODO when model supports it
" or similar. - I'd make the new custom datadir path selected from the file dialog visible in 2 places:
-
under the custom datadir field as it's shown in figma:
-
somewhere in the "Storage settings" page but after onboarding process I think (this definitely as a follow-up) but this needs to be discussed with designers (#bitcoin-core-app channel on "Bitcoin Design" discord) because I don't see any reference in figma (unless I missed it)/ cc: @GBKS. It's important here to mention that would be good also to show here the
-datadir
passed as an argument to QT, which won't be persisted inBitcoin-Qt-*.conf
as the custom data dir, or even the default datadir if that's the case (eg on Ubuntuhome/${user}/.bitcoin
) so the user is aware at all times where the data is being taken/ saved at all times.
-
Already merged, but I just tested on Android and MacOS and the UI works fine. Nice work on this, sounds like file picking was quite the heavy lift to get done. I agree with the UI proposals up there and will prep respective designs. |
I created a follow-up issue for the directory display in storage settings here. Would be great to get your input on the question I have in that issue. |
7a8fb19 qml: UI only display datadir functionality (D33r-Gee) b648cbb qml: added getting custom datadir for display (D33r-Gee) Pull request description: This pull request builds upon #392 and introduces enhancements to display the data directory information within the UI. This functionality encompasses both default and custom data directory paths, fulfilling the UI requirements for user-defined data directory selection initiated in #273. Also the custom datadir is not persistent at the moment it will be once the back end wiring is added. <details> <summary>Ubuntu 22.04 Screenshots</summary> ![datadir_desktop](https://github.com/bitcoin-core/gui-qml/assets/111142327/639873a5-fd5d-44ac-b0be-66e0762a08db) </details> <details> <summary>Android Screenshots</summary> ![datadir_mobile_720](https://github.com/bitcoin-core/gui-qml/assets/111142327/e6fcd12b-f6e6-4efc-adba-071d2caaddef) </details> As a potential follow-up enhancement, consider incorporating mechanisms for saving the data directory path. This could be achieved through: - Double-click functionality: Allow users to save the displayed path by simply double-clicking on it. This provides a convenient and intuitive method for desktop environments. - Dedicated button: For mobile use cases or scenarios where double-clicking might not be feasible, introduce a dedicated "Save Path" button. This ensures a clear and accessible action for users on various devices. ACKs for top commit: GBKS: ACK 7a8fb19. Looks and feels great, just tested again on MacOS. MarnixCroes: tACK 7a8fb19 on Ubuntu desktop pablomartin4btc: reACK 7a8fb19 Tree-SHA512: 4be20832b72fee99046e78ce45e766ff3f00fc556d57c47311828daa3270d1980fdb2b16b95715024a5835f5730f4a3a347337e9dcc22f57fbaf2ac6711a9f6d
This pull request is a focused iteration of #390, intended to isolate and test the UI frontend elements. Backend functionality has been intentionally excluded to streamline the testing, review, and merge process.
Ubuntu Screenshots
Prerequisites
For testing this pull request, ensure you have the following Qt modules installed:
depends/aarch64-linux-android
) and rebuild them.Implementation Details
This introduces
FileDialog
class for a user-friendly selection experience.options_model
files to account for custom data directory configuration and placeholders for customdatadir
wiring.StorageLocations.qml
to allow for custom data directory selection.Follow up PR will add display of the custom
datadir
in StorageSettings.qml