-
-
Notifications
You must be signed in to change notification settings - Fork 294
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
[bukuserver] Bookmarklet, optional fetch, refactor #643
Conversation
Thank you! @rachmadaniHaryono please review when you get the time. Guys, please let me know when you want me to make a release. If you want some more time to test (and get it tested by users as well) no problem. Also, feel free to enrich the GUI. If it can be made more visually appealing without sacrificing performance, we can discuss. It seems users are very much interested in the GUI, thanks to @rachmadaniHaryono for coming up with the idea of |
Er… I thought the review was supposed to be done before the merge 😅
Well, when I have a UX suggestion I usually create a feature request ticket for discussion (although come to think of it I haven't had access to label management in most GitHub projects I worked with so I kinda forgot to add any 😅), such as was the case with delete button on the bookmark edit page; as for a visually appealing UI design, I don't have much ideas as I'm no designer – the only suggestion I have so far is implementing a "dark mode" (whether as a UI setting or via a variable), possibly as a default setting. (For a dark theme, out of these free ones on MIT license, one of the following could be used: Cyborg, Darkly, Slate, Superhero… Slate seems to have a softer contrast while retaining a similar look) edit: added an issue for dark mode discussion
Dunno about testing, but I'm pretty sure it'd be a good idea to complete #637 (documenting the API) before making a release… Unless you want to make another release soon after, just to make the API "official" (it would hardly make sense to "release" the API in-between the app releases). edit: just found another bug, albeit a minor one 😅 |
it is more hassle to revert the pr so i will review it on next pr
i agree with @LeXofLeviafan that api doc should be done before release also because this is big release for bukuserver, warn the user that anything may break |
If we're going ahead with #648 (and possibly other API changes), it should probably also be done before release. |
Several commits with co-dependent code.
fixes #639:
create_form()
create_model()
(producing custom error message)fixes #642:
note: URL parser works differently in JS, so I'm using the netloc value calculated in Python
also:
get_one()
, as suggested in Bookmarklet handling form in bukuserver ignores entered values #639get_bool_from_env_var()
(and documenting valid inputs in readme)note: there's an upstream bug occurring in certain circumstances which breaks backlink logic; I suggest waiting for an upstream fix rather than trying to work around it
flash.js
) out into a macro library