-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Show only related tags, select multiple tags #1775
Conversation
I just changed to use |
dc3bece
to
15955f4
Compare
69c3f8c
to
9f21192
Compare
If it's ok, then I'll try to write tests. |
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.
please confirm my review!
browser/main/index.js
Outdated
@@ -73,6 +73,9 @@ ReactDOM.render(( | |||
<IndexRedirect to='/alltags' /> | |||
<Route path=':tagname' /> | |||
</Route> | |||
<Route path='narrowToTag'> |
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 is this for?
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 forgot to remove, thanks for pointing it out!
LGTM:smile: |
Should I write tests in a different PR or this one is not finished without those? |
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.
Sorry, I forgot about tests.
please write tests in this pr.
I'm so sorry, but it's too hard for me: Boostnote is the very first React / Electron / whatever project for me and I'm just a beginner in the Javascript world, thus writing such tests is nearly impossible. All what I did is modifying Jest test. |
@bimlas I'll take care of it. 😄 |
Hi @bimlas , thanks for your contribution! There is conflict, could you fix it? |
When a tag is selected, the tag list narrows to show only the related ones: all tags associated to the currently visible notes. Clicking on the plus sign near another tag narrows the list again to the tags of notes associated with the firstly AND secondly selected tag. To show every tags again, press the tag icon on the top-left corner of Boostnote. Before: ![screencast](https://i.imgur.com/PwAdhLe.gif) After: ![screencast](https://i.imgur.com/s3JCaFq.gif) NOTE: Tags are joined with `&` character (`#` not works) in `location.pathname` thus it will make the tags with this character unavailable. Any suggestion to pass multiple values via pathname?
Using `&` to separate tags in path (like `/tags/currently&selected&tags`) may interfer with tags including `&` character (like `black&white`). Since ` ` is replaced with `_` when adding tag to notes, it's ideal separator because it's guaranteed that tags are not including this character.
It's easier to understand by most of the users. Later I like to add a setting to enable narrowing of tag list to show only the related ones.
86df09e
to
c4ec69a
Compare
Rebased, fixed. |
Merged. Thank you for your contribution @bimlas ! |
When a tag is selected, the tag list narrows to show only the related
ones: all tags associated to the currently visible notes. Clicking on
the plus sign near another tag narrows the list again to the tags of
notes associated with the firstly AND secondly selected tag. To show
every tags again, press the tag icon on the top-left corner of
Boostnote.
Before:
![screencast](https://camo.githubusercontent.com/7ef55e85fd45dfc3c358975b835637cd4e4e70ad5328e5668825cde80760c046/68747470733a2f2f692e696d6775722e636f6d2f50774164684c652e676966)
After:
![screencast](https://camo.githubusercontent.com/4e5cfd4ee88c2724eb67488ab74885263fff6444bbab93d0b2e0a0ef55d7f940/68747470733a2f2f692e696d6775722e636f6d2f73334a436146712e676966)
NOTE: Tags are joined with
&
character (#
not works) inlocation.pathname
thus it will make the tags with this characterunavailable. Any suggestion to pass multiple values via pathname?
Please check browser/components/TagListItem.styl, I'm sure that it can be simpler.
Related issues: