Few major changes (sorry for not dividing into multiple PR) #27
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
New Version 1.9.0 with few more features : Cards View (Beta) V1.9.0 |
Cards View (Beta) v1.11.0Cards View (Beta) v1.11.0🥳New Features 🆕
Bug Fixes 🛠
Other Changes
Full Changelog: tu2-atmanand/obsidian-notes-explorer@1.10.1...1.11.0 |
|
The only thing required most to improve the UI of the view is to add css to the content rendered inside the cards. The content is actually generated by Obsidian APIs, hence its looks kind of similar to the one you see in the Live Preview mode of notes. I said kind of because there are some wird UI elements. |
|
this is brilliant |
|
|
|
My bad, it was a silly mistake. It's working now with BRAT. |
|
Hello to all ! Whoever has been following my fork of this plugin. After considering the suggestions from few of the users, and as I went on adding various new features in my version of this plugin. IT has become an entirely different kind of plugin, because of these changes. (Although the idea and sort of working has been same, as of now atlease, which In future I am planning to change too). So keeping all these points on table. I have decided to publish my own version with a new name to the Obsidian Marketplace, so users can install it easily. Therefore, it won't be possible for me to continue the contribution to this main plugin through different PRs. But Cards View is always welcome to integrate this PR. You'll can check out the new plugin here : Notes Explorer Contributions and suggestions are always appreciated! |
There was a problem hiding this comment.
This PR was mentioned on Discord, so I thought I would have a quick look at it, and see what would be involved in reviewing it.
When reviewing, one needs to compare the original code and the new code, and go through each difference, understanding the purpose of the change, and checking that the owner could still maintain the code.
Because this commit includes moving source files around, the GitHub UI shows virtually no diffs at all - mostly just a bunch of totally deleted files, and another bunch of newly created files.
There is also a lot of reformatting going on.
My personal opinion - as an experienced maintainer of other open-source projects - is that this PR should be closed and not merged...
And instead a conversation should be started asking @jillro for the OK to rearrange the source directory structure, for example, and if agreed then creating a PR that does only that one thing.
|
Another approach is to review each of the commits, one at a time... So I looked at the first commit, and it is:
But there is no information in the commit message about what the bug fix was - and because the commit contains both structural changes and a supposed bug fix, it is not possible to simply review the diff and try to spot the bug fix. This suggests that reviewing the remaining 44 commits may not be a good use of time. |
|
Hi @claremacrae! Thank you so much for giving your time for reviewing this PR. I really appreciate for your key points. I always learn so much from other developers. Likewise, I would like to apologize for this PR, which I also have done in the title of the PR as well. From my past development experience, I got the habit of working on achieving the functionality which I have currently in my mind and just putting it on the compute before it evaporates from my mind. And hence I focused less on giving proper comments on the changes. In the organization, we use to directly suggest for the changes through massive PRs like this and expect that the author will go through the changes and can integrate the code into the main repository and then make the various changes required before publishing. Also, we had a separate workflow, where there used to be a separate branch other than the main, where contributors should propose their changes, and then the author will fine-tune the code by making any additions or deletions to the merged code. Once everything has been working fine, the author can then merge the new changes in the main branch for publishing. I assumed the same thing, and also because my intention was to implement what I had in mind rather than contributing to the plugin, I went on writing the code, before creating the PRs, hence made a compiled version of the PR, in the hope that, the author will look into my changes and will only keep the required changes rather than simply merging my code as it is in the production branch. But after going through the discussions from discord and from the above suggestions, I have got a better view on having a friendly approach for contributing on different types of GitHub projects, so we don't upset anyone. |
|
So, I would request @jillro to please create a new issue on this same topic so we can discuss these things further before I create a new PR. And can close this specific PR. I'll try to submit better PRs for each specific features I have implemented. And also your thoughts on organizing the codebase into src folder ? |
|
Hi @tu2-atmanand, I'm glad the time spent was useful...
That seems to me the wrong way round, asking the maintainer to make the effort to start the discussion... Better for you, @tu2-atmanand, to create a new issue, say on the single topic of moving all the existing source code in to a You would need to include some information on why you feel it is a good idea, and what the benefits would be. And if you do get the go-ahead, you can then demonstrate that you are able to make single-topic PR that is easy to review. For example, don't even think of updating |
|
Yes, I agree with you. But still, I'll go with your suggestions. How about this, I'll keep this PR open for @jillro to add her comments on this whole discussion and then let her close it. So she will have the context of the new Issue I'll be creating. |
|
To my first baby-step 🍻 : #28 |
|
@tu2-atmanand @claremacrae I initially wrote this in a separate thread, but it seems that this thread is where most of the meta-discussion about the plugin is taking place, so I’ll write here in length. First, a quick disclaimer: I come from a very different branch of r&d (algorithms and data science) and have limited knowledge of the specific tech stack involved, which is why I haven’t contributed directly so far. When a new piece of open-source software like this comes along that fulfills a much-needed function, there is often a rush to add new features quickly and close gaps. While this is understandable, it can sometimes hinder the long-term viability of the project by bypassing important best practices. We all share the goal of seeing this plugin grow, mature, and remain stable, avoiding the fate of many projects that fizzle out after a few months or years. In my view, the only way to ensure this happens is by establishing clear contribution guidelines and project governance. Specifically, there should be clear, transparent guidelines for how to contribute to the project, including processes for submitting issues and PRs or proposing new features. This would ensure contributions are aligned with the project’s vision and help maintain high-quality standards and avoid messy CRs as above. Contributors should know what the expectations are and how they should engage with the project. For this reason, I believe that splitting this plugin into separate projects, bypassing communication and decision-making is the wrong idea. Instead, we should focus on building a strong community around this plugin, where open communication, shared ownership, and collaboration will see it reach #1 on the community plugins :) If @jillro is unable to dedicate enough time to all this, it would be helpful to add additional maintainers to share the workload. |
|
Hello @tu2-atmanand and others ! Although I recognize this amazing coding efforts made here, I will not merge this PR, mainly for the reasons detailed by @claremacrae and @yuvgin. Major changes, such as new features and/or changes in the code structure should ideally be discussed and agreed upon before submitting a PR. There is too much different things in that PR for me to take the responsibility to merge them : every merge is a commitment to maintenance in the long run. It is not a a decision to refuse all changes of this PR, but all of them should be discussed and considered separately. Nonetheless, if you want take this plugin in a different direction than this one without waiting, you're absolutely free to create and maintain your fork.
I agree 100% with this, and I'll write contributing guidelines soon ! |
I have implemented few major changes, as mentioned below. I know, I should have divided them into small PR for each feature, perhaps you can integrate this PR as you want, or simply using copy and paste, old-fashioned.
Whats Changed
Updated the project structure, moved everything inside src/ directory, let's follow a standard structure.
BUG FIX : I don't know about other users, but I was facing an issue, where the overflow was unset for the
view-contentdiv which is Obsidian's parent div for View, in which the Card View gets rendered. Hence, I was not able to scroll. I have moved the scrolling logic inside thecard-container, so only the section of cards are scrolling and the header will be sticky on the top.Setting option to hide the delete button.
The buttons in the footer are only shown on card hover, to give a clean look for all the cards.