Skip to content
This repository was archived by the owner on May 12, 2026. It is now read-only.

Show only related tags, select multiple tags#1775

Merged
kazup01 merged 8 commits into
BoostIO:masterfrom
bimlas:narrow-to-related-tags
Apr 26, 2018
Merged

Show only related tags, select multiple tags#1775
kazup01 merged 8 commits into
BoostIO:masterfrom
bimlas:narrow-to-related-tags

Conversation

@bimlas

@bimlas bimlas commented Apr 1, 2018

Copy link
Copy Markdown
Contributor

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

After:
screencast

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?

Please check browser/components/TagListItem.styl, I'm sure that it can be simpler.

Related issues:
@Rokt33r Rokt33r added feature request 🌟 Issue is a new feature request. awaiting review ❇️ Pull request is awaiting a review. and removed feature request 🌟 Issue is a new feature request. labels Apr 2, 2018
@bimlas

bimlas commented Apr 7, 2018

Copy link
Copy Markdown
Contributor Author

I just changed to use character to separate tags in path (/tags/currently selected tags) instead of & (/tags/currently&selected&tags) to prevent interference with tags containing this char (black&white). See commit message. I think it's stable enough to get merged.
@sosukesuzuki sosukesuzuki mentioned this pull request Apr 10, 2018
Closed
@bimlas bimlas force-pushed the narrow-to-related-tags branch from dc3bece to 15955f4 Compare April 10, 2018 13:21
@bimlas

bimlas commented Apr 10, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Changed the behaviour: does not narrowing the list.

screencast
@bimlas bimlas force-pushed the narrow-to-related-tags branch 2 times, most recently from 69c3f8c to 9f21192 Compare April 11, 2018 06:42
@bimlas

bimlas commented Apr 11, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Just made the final version:

screencast
@bimlas

bimlas commented Apr 11, 2018

Copy link
Copy Markdown
Contributor Author

If it's ok, then I'll try to write tests.
@bimlas bimlas mentioned this pull request Apr 11, 2018
Closed
@bimlas bimlas changed the title Narrow list of tags to related ones Apr 11, 2018
@sosukesuzuki sosukesuzuki self-requested a review April 11, 2018 09:10
sosukesuzuki
sosukesuzuki suggested changes Apr 11, 2018
View reviewed changes

@sosukesuzuki sosukesuzuki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please confirm my review!
Comment thread browser/main/index.js Outdated
<IndexRedirect to='/alltags' />
<Route path=':tagname' />
</Route>
<Route path='narrowToTag'>

@sosukesuzuki sosukesuzuki Apr 11, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for?

@bimlas bimlas Apr 11, 2018

Copy link
Copy Markdown
Contributor Author

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!
@sosukesuzuki

sosukesuzuki commented Apr 11, 2018

Copy link
Copy Markdown
Contributor

LGTM:smile:
@bimlas

bimlas commented Apr 11, 2018

Copy link
Copy Markdown
Contributor Author

Should I write tests in a different PR or this one is not finished without those?
sosukesuzuki
sosukesuzuki suggested changes Apr 11, 2018
View reviewed changes

@sosukesuzuki sosukesuzuki left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

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.
@bimlas

bimlas commented Apr 11, 2018

Copy link
Copy Markdown
Contributor Author

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.
@Rokt33r

Rokt33r commented Apr 12, 2018

Copy link
Copy Markdown
Member

@bimlas I'll take care of it. 😄
@sosukesuzuki sosukesuzuki removed the awaiting review ❇️ Pull request is awaiting a review. label Apr 24, 2018
@kazup01 kazup01 added the awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. label Apr 26, 2018
@kazup01

kazup01 commented Apr 26, 2018

Copy link
Copy Markdown
Member

Hi @bimlas , thanks for your contribution! There is conflict, could you fix it?
bimlas added 5 commits April 26, 2018 15:38
@bimlas
Narrow list of tags to related ones
6c54275 
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?
@bimlas
Separate active tags by instead of &
68c0f21 
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.
@bimlas
Don't narrow list, add plus sign only for related tags
61ed47d 
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.
@bimlas
Cleanup, prettify
066d972
@bimlas
Simplify style
c02b91d
@bimlas bimlas force-pushed the narrow-to-related-tags branch from 86df09e to c4ec69a Compare April 26, 2018 13:39
@bimlas

bimlas commented Apr 26, 2018

Copy link
Copy Markdown
Contributor Author

Rebased, fixed.
@kazup01 kazup01 added the awaiting review ❇️ Pull request is awaiting a review. label Apr 26, 2018
@sosukesuzuki sosukesuzuki removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. awaiting review ❇️ Pull request is awaiting a review. labels Apr 26, 2018
@kazup01 kazup01 merged commit b191213 into BoostIO:master Apr 26, 2018
@kazup01

kazup01 commented Apr 26, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Merged. Thank you for your contribution @bimlas !
@bimlas bimlas deleted the narrow-to-related-tags branch May 4, 2018 21:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

4 participants

@bimlas @sosukesuzuki @Rokt33r @kazup01