Skip to content
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

Initial attempt to fix #560 #905

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

deepkn
Copy link

@deepkn deepkn commented Jun 27, 2024

Enables decorations for user specified tags - currently supporting only a single decoration - a 'prefix' to be added to the page name. Prefix is handled in the top bar title, page navigator as well as links.

Since visually identifying my defined object types was somewhat important for me, I took a stab at #560 - Quite unfamiliar with typescript and in general PWAs - so there could be a lot that can be improved and even better ways to do this. Open to suggestions.

TODO: Handle this in queries as well, at least when rendered via templates - but I'm wondering if that could be achieved better by simply extracting out the prefix as an attribute and using that (leaving it to the user)

deepkn and others added 4 commits June 28, 2024 01:19
Enables decorations for user specified tags - currently supporting
only a single decoration - a 'prefix' to be added to the page name.
Prefix is handled in the top bar title, page navigator as well as
links.
@zefhemel
Copy link
Collaborator

zefhemel commented Jul 2, 2024

This looks promising. Definitely a good starting point to a more elaborate implementation. I'll have a closer look soon!

@zefhemel
Copy link
Collaborator

zefhemel commented Jul 3, 2024

A few issues I've found:

  • I'm not sure the prefix should actually become part of the page name in the top bar. I think it should be put in front of the editor component somehow so you cannot edit it out. Right now, you can simply remove the prefix there, which is a bit weird.
  • The prefix is only added in the top bar page title when you navigate from another page to it, not when the page is loaded refresh (reload the page and you'll see what I mean)
  • The prefix is only rendered for wiki links when they're out of the selection scope. I think may be ok, although it would be slightly nicer if it would also be there in the code complete as well as when the cursor is inside the link:

CleanShot 2024-07-03 at 07 48 59

Some of these issues may be harder to get done. If you get stuck or you'd like me to take over let me know. This is a good start, and this is definitely something I'd like to get in soon, so happy to help!

@deepkn
Copy link
Author

deepkn commented Jul 4, 2024

Thanks Zef. Happy to take another shot at it - after all I'm loving SilverBullet a bit too much :)

  1. That makes sense - I'm guessing that would have to go as another component for prefix before the MiniEditor; do correct me if I'm completely off here.
  2. I did see this happening last day - I mostly never leave keyboard while navigating(one of the things I love about sb) so that missed my eye. I think I know what's happening there - infact I think the very first commit would not have had this problem. Let me think of a cleaner fix.
  3. Sounds like a little bit more trickier to do - atleast based on what I have explored so far, so any pointers welcome. But I'll still take a shot at it in the coming days.

1. The decorator prefix being an editable component in the top bar.
   It is now rendered in it's on container, separate from the
   MiniEditor and is hence not editable by the user. This also
   means we need no custom logic in the topbar page rename code.
2. The decorators not being applied on page reload. The page list
   gets updated post the "page-loaded" event in most cases. Hence
   if the current page gets a prefix, we need to force a render.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants