-
Notifications
You must be signed in to change notification settings - Fork 896
WIP: Add shape library plugin #6020
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
base: master
Are you sure you want to change the base?
Conversation
7251a52
to
7990087
Compare
7990087
to
fa9020a
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.
Just some comments I had when reading over this. Nothing severe though.
Oh and I really like idea of this plugin (didn't test it yet though). If there's anything I can do to help, let me know.
plugins/ShapeLibrary/README.md
Outdated
## Add Your Own Shape with "Extract_Stroke_Info" | ||
|
||
1. **Draw your own shape** in Xournal++ and place it at the top left corner of the page. | ||
*Note: The shape will be inserted at the same position. When you want to insert the sape if the page is smaller than the current one, you may not be able to see the inserted shapes.* |
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.
typo: sape -> shape
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.
Well we could also do this in the plugin (placing the icon at the top left corner by translating the coordinates) and then on insertion e.g. place the icon at the center of the page (or viewport if this is possible).
A bit sad that we currently don't have a way of getting something like pointer coordinates for plugins. This would then be going into the direction of making custom tools via plugins. But this is way outside of the scope of this plugin (maybe just something to keep in mind for the future).
Maybe related: #4703
if app.addToSelection then | ||
app.addToSelection(refs) | ||
else | ||
print("Cannot add shape to selection because of missing API, consider upgrading Xournal++") |
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.
I like this safeguard (eventhough most users won't see it as usually they probably won't run xournalpp from the console)
plugins/ShapeLibrary/stroke_io.lua
Outdated
file:write("}\n") | ||
file:write("return strokesData") | ||
file:write(" -- Return the strokesData table") -- End of strokesData table |
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.
is using multiline strings "better" here? (probably it doesn't really matter)
e.g.
file:write([[}
return strokesData -- Return the strokesData table]])
46c91f8
to
31077d4
Compare
I'd be interested in having a look once this reaches a somewhat final state (always not sure about the current status with this setting of this PR here and the remote repo too). So don't hesitate to shoot me a message here. |
There's still quite some work to be done. Feature parity with the remote version is not yet reached. I'll let you know once it's reasonably complete. |
267d926
to
52fb8ac
Compare
The plugin now has most features built in that https://github.com/MiltonBalaOfficial/ShapeLibraryWithLgi has and adds some more. Installing it on the system (and adding shapes to a user folder) is not yet supported. Here is a demo: ShapesLibraryDemo.mp4 |
Not sure how you fast you want to get this done now. Sadly I've got not too much time right now, I'll probably only able to give this a more detailed look in the end of next week. |
It will take more time than that for sure. I won't have too much time to work on it in the next couple of days. |
Co-authored-by: MiltonBalaOfficial <118467836+MiltonBalaOfficial@users.noreply.github.com>
@co-authored-by: MiltonBalaOfficial <118467836+MiltonBalaOfficial@users.noreply.github.com>
efda9b4
to
43234f8
Compare
Now there are system categories (which are read only; they can be hidden via a check button) and user categories (which you can freely modify; they can also be hidden via a check button). The system categories are only read once, since they shouldn't change. For the user categories the Some cleanup is still needed, but mostly all functionality is there now. |
It's great to see the official plugin almost ready! I've taken a look at it, and here’s my feedback:
|
I agree, that would simplify matters for the user. The plugin could generate a file name from the shape name and if the file already exists, it could modify the filename so that it becomes unique.
Yes, it would be possible, but it would complicate things internally quite a bit, I fear. Maybe it's better to add an option to duplicate an existing (system) category. What do you think about that?
Sure, also the README.md has to be refined.
Thanks! By the way, one shape I can't find much use for is the Border Booklet (in the Page Decoration category). What page format is it supposed to be good for? I usually have A4 (portrait) and there it's too large to fit onto the page. |
Since it was designed for a specific 16:9 setup, it may not be generally useful. I think it should be removed from the official plugin, as it is user specific.
Yes, that will be fine. This is what I meant earlier but couldn't express clearly. After duplication the user might uncheck the system categories. If it is possible to auto-duplication and auto uncheck in the first use it would be great! |
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.
Do you use an "IDE" which can make sense of LuaLS annotations? If so, maybe it would make sense for this a bit larger plugin to add those annotations to the functions (together with some doc-comment like annotations)
Also note, that (as I mentioned) I'm really no one you should ask about UI stuff so I didn't really review the lgi_dialog.lua
file (just wanted this to note).
I also added some bug reports (this way we have a thread for an organized conversation and can resolve the thread when the bug is fixed) which are not solved yet. Now I just tested the plugin a bit, but don't worry I also want to help here looking deeper into the bugs in order to fix them (just not today anymore)
Oh and I really didn't want to forget this, but still I did :/
I really love the shapes, nice job creating them @MiltonBalaOfficial (at least as far as I'm aware, this was you)
@@ -0,0 +1,4 @@ | |||
label { | |||
color: orange; |
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.
Tbh the orange looks a bit odd (but this is opinion based). Can't we somehow refer to the user's GTK Style here?
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.
What do you mean by the user's GTK style?
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.
The thing you set via a system dialog or e.g. lxappearance
. Something like Adwaita
or Arc-Darker
.
@@ -0,0 +1,616 @@ | |||
local _M = {} | |||
|
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.
Also as a user it was not intuitive that new User cat.
are not directly shown when adding them via the UI (from a code viewpoint it might make sense of course, but we'd need to communicate this somehow).
But then this also might be a bug since after a restart (of xournalpp), the new User cat.
was present, but when adding a new User cat.
the old User cat.
disappeared again.
SImilar: When (after a restart) trying to add a shape to one of the User cat.
, all the User cat.
disappear again but is present (can even be inserted) after another xournalpp restart.
Doesn't this happen for you too?
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.
That must be a regression. By the way ticking the "Show user cat" off and on makes the categories appear.
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. 4D32 p>
It should be fixed now with commit a2a6fa3
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.
Maybe related (similar "trigger"):
Still at least for me:
- disable
system cat
-> no categories shown - add a custom
user cat
- results in
system cat
s being shown again
Same when adding a new shape to a user cat
with the system cat
s disabled
Also an idea from me: Inserting shapes is inserting individual strokes. As a result the user can modify the shape afterwards (which I find pretty cool btw). But this also means the user might not be able to select the individual shape anymore. The solution for grouping strokes currently is putting them on an individual layer. Maybe adding a checkbox which leads to |
As per #6008 (reply in thread) this is @MiltonBalaOffical's Quick shape library plugin from https://github.com/MiltonBalaOfficial/Quick-shape-library with a bit of polish. More of it is needed, although the plugin is already in a usable state.