Add a search box on the navigation bar #255
Conversation
suprith-hub
changed the title
|
Oh, this is already looking pretty gorgeous. Great job @Era-cell 
|
assets/main.js
Outdated
| const item = document.createElement('li'); | ||
| item.innerHTML = `<a class="dropdown-item" href="${result.item.permalink}"> | ||
| ${result.item.title} | ||
| <span style="font-size: 0.8em; color: #888;">(${result.item.dialect})</span> |
There was a problem hiding this comment.
Can we use Bootstrap classes here instead of an inline style? I think Bootstrap has the classes you need for what you are doing here
There was a problem hiding this comment.
For the color I got closer colors, which I can use. For font-size there is sizes for different headings but I couldn't find for small variations
Yeah, maybe try an input group with the search icon? https://getbootstrap.com/docs/5.3/forms/input-group/. We already have https://icons.getbootstrap.com available on the site for you to use |
|
We need to some work on how this looks on smaller screens, but I would leave that to the end, once everything else is set in stone 
|
assets/main.js
Outdated
|
|
||
| function executeSearch(searchQuery) { | ||
| // TODO | ||
| // show(document.querySelector('.search-loading')); |
There was a problem hiding this comment.
Do we need this? Seems pretty instantaneous to me
There was a problem hiding this comment.
Maybe not right now, I don't know how fast it may work when there are all the dialects present. Maybe i'll just have a statement here to remind it?
assets/main.js
Outdated
| } | ||
| }); | ||
|
|
||
| document.addEventListener('click', function(event) { |
There was a problem hiding this comment.
| document.addEventListener('click', function(event) { | |
| document.addEventListener('click', (event) => { |
|
Already left some comments but man, this is looking great already. Very, very cool |
|
Now that I look at it, Bootstrap has this pretty search icon embedded in the input. I wonder how they do it 
|
|
Also, just for design brainstorming purposes, expanding the search bar doesn't look that bad. What do you think? 
|
For now the input-group worked, the icon inside of the text-box isn't working. I will have a PR. |
|
@Era-cell Looks really cool already, but note the vocabulary and dialect name are now invisible on light theme: 
|
|
Check this out: 
It works well in both light & dark if you do the following: diff --git a/assets/main.js b/assets/main.js
index 0465119..f545508 100644
--- a/assets/main.js
+++ b/assets/main.js
@@ -68,8 +68,8 @@ const updateDropdown = (results) => {
const item = document.createElement('li');
item.innerHTML = `<a class="dropdown-item" href="${result.item.permalink}">
${result.item.title}
- <span class="text-white-50" style="font-size: 0.8em;">(${result.item.dialect})</span>
- <span class="text-white" style="font-size: 0.9em;">${result.item.vocabulary}</span>
+ <span class="text-light-50" style="font-size: 0.8em;">(${result.item.dialect})</span>
+ <span class="fw-bold" style="font-size: 0.9em;">${result.item.vocabulary}</span>
</a>`;
box.appendChild(item);
});
@@ -77,7 +77,7 @@ const updateDropdown = (results) => {
const item = document.createElement('li');
item.innerHTML = '<a class="dropdown-item disabled" href="#">No matches found</a>';
box.appendChild(item);
- }
+ }
} |
assets/main.js
Outdated
| } | ||
| }); | ||
|
|
||
| document.addEventListener('click', (event) => { |
There was a problem hiding this comment.
I think you can get rid of this. I get the idea of closing automatically, but I think we should let HTML do its default job!
There was a problem hiding this comment.
What is the use of this function is user can come out of the search without navigating to other page
temp2.mp4
temp11.mp4
|
Finally, this is really cool and I'd like to ship this ASAP. Let's think about mobile support on a different PR, and just add the following to hide the search bar on mobile for the time being: diff --git a/layouts/partials/navigation.html b/layouts/partials/navigation.html
index e5dec5a..3e9708b 100644
--- a/layouts/partials/navigation.html
+++ b/layouts/partials/navigation.html
@@ -5,7 +5,7 @@
height="42" width="42" class="me-1">
<span class="align-middle">JSON Schema Docs</span>
</a>
- <div class="position-relative me-3 flex-grow-1">
+ <div class="position-relative me-3 flex-grow-1 d-none d-lg-block">
<div class="input-group">
<span class="input-group-text" id="basic-addon1">
<i class="bi bi-search"></i> |
Done 👍 |
|
@Era-cell Can you resolve the conflict? |
jviotti
changed the title
|
I have resolved it 👍 |
|
@Era-cell CI seems to be complaining about the JavaScript file:
|
|
Okay, seems good now. The power went off yesterday 😅 |
|
Thanks a lot! |
Thank you too, website looks very good 💖 |
@jviotti review on this PR and can we better the search bar, I feel we should have the search logo 🤔