-
Notifications
You must be signed in to change notification settings - Fork 382
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
<FilePicker/> Add properties for default library- & folderPath for <SiteFilePicker/> #936
Comments
Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible. |
For anyone interested I've started working on this here, I've added one commit for the initial logic of parsing the string property "defaultFolderAbsolutePath". Next up is to add everything to the breadcrumb to preserve current functionality. Feel free to comment. |
@Katli95 Both are great suggestions! |
@joelfmrodrigues awesome! Thank you, no need for help as is, just a question: do you have an idea of how long it may take to get this released once I submit? I'm working on a project atm that requires this feature 😇 |
It should not take long to review the PR, and after it's merged a new beta version is automatically created and published to npm so you can install it and then update later to a major release. |
Awesome, glad to hear it! How free should I feel to generally refactor the code? (Nothing major ofc, but somehting like changing variable names and removing obsolete code? |
@Katli95 Ideally, try to only modify the code that is relevant and necessary to what you are implementing. This is a common approach with many open source projects as otherwise code reviews would be more complex and someone else implementing features on related code could be impacted and have to deal with code conflicts, etc. If for example, you move a block of code, it's all marked as a change in the code review and impossible to identify if something changes unless we compare line by line. Even something simple like letting VS Code format an entire document can cause a lot of changes in the pull-request and is discouraged. Please check the Do's and Dont's section of the documentation: |
Awesome, ok, good to know, hahah yeah, first thing I did was to disable prettier. And yeah, I looked through those before I began. I was mainly thinking of renaming state variables to make their intention clearer as I had some trouble understanding them at first and also removing some which don't seem to be used anymore. But I'll restrain myself and let it lie 😅 Should submit the pull request this weekend 🙌 I've finished most of the actual work, just have to add the docs and learn to rebase. If you have the time I'd be grateful if you could look at it and let me know if I'm doing something wrong, if not ofc I understand. |
Well, this is a bit of a different story then 😊 |
My pull request has been merged, so I'm closing the issue :) |
Please don't close until the version is released. |
Oh, sorry, didn't know that's how this works ✌️ |
Category
Version
3.1.0
Desired Behavior
It would be great if the
<FilePicker/>
could accept a property/properties for setting a default selected library and folder which would apply to the<SiteFilePicker/>
child.Implementation
I'm up for submitting a Pull Request and I might even take a look into #725 while I'm at it as a multiple file select is applicable for my use case as well.
Right now I'm thinking of just giving a single property named
defaultFolderAbsolutePath
(perhaps with a suffix to indicate its intention for the SiteFilesTab) and parse it in the<SiteFilePickerTab/>
constructor for the state variables (and breadcrumb ofc).The text was updated successfully, but these errors were encountered: