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

Added blacklist feature to tag filter. #52

Merged
merged 6 commits into from
Jul 15, 2020
Merged

Conversation

natask
Copy link
Contributor

@natask natask commented Jul 7, 2020

implements #48

@goktug97
Copy link
Member

goktug97 commented Jul 7, 2020

I will look into it as soon as I got time. Sorry, couldn't replied to your #48 issue.

  • Can you use camelCase in variable names.
  • Can you reindent #reload-button {
  • .darkmode .select2-container--default .select2-selection--multiple .select2-selection__choice is already present can you remove it from your PR.
  • Please format the code using indent-region

index.html Outdated Show resolved Hide resolved
@@ -135,6 +138,9 @@
-o-transition: background-color 500ms ease-out;
transition: background-color 500ms ease-out;
}
.select2-container--default .select2-selection--multiple .select2-selection__choice {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@natask natask Jul 7, 2020

Choose a reason for hiding this comment

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

The reason why I did this was because a white text looked to have a better contrast on green and red tag backgrounds. It also more consistent with the other text on the page because they are all white. We can merge it to

org-roam-server/index.html

Lines 143 to 145 in a42a355

.darkmode .select2-container--default .select2-selection--multiple .select2-selection__choice {
background-color: #1d1e20;
}

like so.

      .darkmode .select2-container--default .select2-selection--single,
      .darkmode .select2-container--default .select2-selection--multiple,
      .darkmode .select2-search--dropdown,
      .darkmode .select2-results,
      .darkmode .select2-container--default .select2-selection--multiple .select2-selection__choice,
      .select2-container--default .select2-selection--multiple .select2-selection__choice {
          background-color: #1d1e20;
          color: #FFFFFF;
      }

I think it will be better to leave them separate for manageability sake in the future. It will add confusion later on if merged into one css block. background-color:: #1d1e20; won't apply to selection__choice because it is being set to green or red but this may change in the future.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Contributor Author

@natask natask left a comment

Choose a reason for hiding this comment

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

I think we should modify the css so that it is consistent with the other white text on the screen. Applying to .select2-container--default .select2-selection--multiple .select2-selection__choice applies to .darkmode .select2-container--default .select2-selection--multiple .select2-selection__choice but not the other way around.
I actually don't think there is a point in keeping darkmode .select2-container--default .select2-selection--multiple .select2-selection__choice within css block that applies background-color because it is going to be overrid with green or red.

@natask
Copy link
Contributor Author

natask commented Jul 9, 2020

quick question, what is the rational for JSON.parsing and JSON.Stringifying internal objects like filter?

@goktug97
Copy link
Member

goktug97 commented Jul 9, 2020

To search the set using the string hash not with an object hash. When you construct a javascript object using the selected dropdown values it will be unique but if you use a string it will have the same hash value with the previous stringifyed object. I don’t know if there is a better solution but this is simple and works without a problem

  - Fixed bug where the variable tag was aliased within vis.filter.
  - Fixed bug where reload button doesn't preserve filters.
  - Preview mode on by default.
  - Rudimentary support for default white and black list members.
    - Currently achieved by modifying index.html.
    - This will be made available as variables that can be set from
      emacs.
  - remove debug prints.
@natask
Copy link
Contributor Author

natask commented Jul 10, 2020

I think it works quite well now. I am now trying to add so that a default whitelist and blacklist can be configured from emacs. other variables configuring preview on by default or default list type can configured as well with the same system. When I have time, I am planning on looking at how information is being passed from emacs side to this application and pass default variables on startup and reload. what do you think?

fixes new update to org-roam db query titles -> title and select titles -> files.
@natask natask requested a review from goktug97 July 13, 2020 16:15
@goktug97
Copy link
Member

goktug97 commented Jul 15, 2020

Thanks for the PR

EDIT: Please don't add your defaults to the PRs

@goktug97 goktug97 merged commit b16a5de into org-roam:master Jul 15, 2020
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