ARK-Builders / ARK-Memo

ARK Memo is one app for all of your notes: it's aiming to combine plain text, voice and hand-written notes
MIT License
2 stars 3 forks source link

Dedicated title/description input fields #46

Closed tuancoltech closed 9 months ago

tuancoltech commented 9 months ago
kirillt commented 9 months ago

@ShubertMunthali not sure what do you mean, I see that the title is stored into new .ark/user/properties storage as JSON. It seems to allow empty description and/or empty title.

@tuancoltech the feature works good, the only issue is that it's not possible to modify title after initial creation if the content isn't changed. Because [save] button doesn't allow saving same note twice. It shouldn't update the content, but any properties updates should be saved.

tuancoltech commented 9 months ago

@ShubertMunthali not sure what do you mean, I see that the title is stored into new .ark/user/properties storage as JSON. It seems to allow empty description and/or empty title.

@tuancoltech the feature works good, the only issue is that it's not possible to modify title after initial creation if the content isn't changed. Because [save] button doesn't allow saving same note twice. It shouldn't update the content, but any properties updates should be saved.

@kirillt At the moment, if we modify a note's content, a new note will be created instead of overwriting the existing note file. Do we want to duplicate the note files, or overwriting in the same file in case user modify the note's description and/or title?

shubertm commented 9 months ago

@ShubertMunthali not sure what do you mean, I see that the title is stored into new .ark/user/properties storage as JSON. It seems to allow empty description and/or empty title.

@tuancoltech the feature works good, the only issue is that it's not possible to modify title after initial creation if the content isn't changed. Because [save] button doesn't allow saving same note twice. It shouldn't update the content, but any properties updates should be saved.

@kirillt I mean what you just explained, that save button does not work if only properties are edited, it works when content is edited. we need it to work with properties edited alone, or with properties edited together with content. That's what is required in #39

shubertm commented 9 months ago

@ShubertMunthali not sure what do you mean, I see that the title is stored into new .ark/user/properties storage as JSON. It seems to allow empty description and/or empty title. @tuancoltech the feature works good, the only issue is that it's not possible to modify title after initial creation if the content isn't changed. Because [save] button doesn't allow saving same note twice. It shouldn't update the content, but any properties updates should be saved.

@kirillt At the moment, if we modify a note's content, a new note will be created instead of overwriting the existing note file. Do we want to duplicate the note files, or overwriting in the same file in case user modify the note's description and/or title?

If only properties change we should overwrite, but whenever content changes we need a new file. Version tracking in #13 will handle the duplicates properly.

kirillt commented 9 months ago

Yep, @ShubertMunthali is right here. We're missing ability to edit title/description when the content is untouched. We don't need to save the content twice in this case, only update user/properties storage for the current resource id.

tuancoltech commented 9 months ago

Yep, @ShubertMunthali is right here. We're missing ability to edit title/description when the content is untouched. We don't need to save the content twice in this case, only update user/properties storage for the current resource id.

@ShubertMunthali @kirillt Thanks for the idea, guys. I'm making changes to align with this.

tuancoltech commented 9 months ago

@ShubertMunthali @kirillt Please help reviewing again. Title/description are now validated and stored independently from content.

kirillt commented 9 months ago

@tuancoltech Good, but couple of nitpicks:

Steps:

  1. Create new graphical note and save.
  2. Open it and change the title, save.
  3. Open it again and change the description, save.

Result: 3 different SVG files.

$ xmllint --format 2035-514095390.svg > /tmp/a
$ xmllint --format 2035-1319256942.svg > /tmp/b
$ xmllint --format 2035-2093532239.svg > /tmp/c
$ diff /tmp/{a,b}
2c2
< <svg xmlns="http://www.w3.org/2000/svg" viewBox="0.0 0.0 1440.0 2550.0">
---
> <svg xmlns="http://www.w3.org/2000/svg" viewBox="0.0 0.0 1440.0 1535.0">
[kirill@lenovo Notes]$ diff /tmp/{b,c}
2c2
< <svg xmlns="http://www.w3.org/2000/svg" viewBox="0.0 0.0 1440.0 1535.0">
---
> <svg xmlns="http://www.w3.org/2000/svg" viewBox="0.0 0.0 1440.0 1398.0">

For some reason, viewBox property is updated.

tuancoltech commented 9 months ago
  • If we create a note without providing title or description, the empty JSON is written to the properties storage: {"titles":[""],"descriptions":[""]}

@kirillt For the empty JSON issue, we could only handle that from arklib-android. I created an issue for it here: https://github.com/ARK-Builders/arklib-android/issues/137

For the second issue, it's because we are updating the viewBox of current SVG whenever there's size change of the drawing view as per in below screenshot. I'm clarifying this with @ShubertMunthali .

Screenshot 2024-01-16 at 12 40 04
kirillt commented 9 months ago

Thank you @tuancoltech, that's perfect.

kirillt commented 9 months ago

So, I guess it's easy to fix as soon as @ShubertMunthali share some info and we'll merge then.

shubertm commented 9 months ago

We need to set dimensions for newly created note only here. @tuancoltech that's a bug indeed, we need a fix. You can provide it, if yo don't mind

tuancoltech commented 9 months ago

We need to set dimensions for newly created note only here. @tuancoltech that's a bug indeed, we need a fix. You can provide it, if yo don't mind

@ShubertMunthali Definitely, I have fixed it by removing the unnecessary onSizeChanged method. Please re-review @kirillt !