Tami/boolean operators #10
Conversation
auto-complete.css
Outdated
| .chip | ||
| { | ||
| border: 1px solid transparent; | ||
| } | ||
| .chip:hover { | ||
| border: 1px solid black; | ||
| } |
There was a problem hiding this comment.
You can't override mystique classes here, this will apply to all chips in the page, replacing the style everywhere.
Create a class for this purpose, like autocomplete-chip.
auto-complete.js
Outdated
| window.changeText = () => { | ||
|
|
||
| const newText = | ||
| ` |
There was a problem hiding this comment.
Do not use ES2015+ syntax features, like arrow function, const or template string. Doing this will break the whole JS script for older browsers, all code from the Topbar JS will not work.
Use ES5 syntax.
auto-complete.js
Outdated
| @@ -62,6 +62,44 @@ | |||
| }); | |||
| } | |||
|
|
|||
|
|
|||
| window.changeText = () => { | |||
There was a problem hiding this comment.
Avoid using global object window for this, it's considered bad practice because of the unscoped access, you could easily replace this function in another place and will break this code.
Instead, create a local function
function changeText() { ... }And add it using JS
var buttons = document.getElementsByClassName('autocomplete-boolean-button');
for (var i = 0; i < buttons.length; i++) {
var button = buttons[i];
button.addEventListener('click', changeText);
}
auto-complete.js
Outdated
|
|
||
| const newText = | ||
| ` | ||
| <div style="margin: 0.4em" class="row"> |
There was a problem hiding this comment.
margin: 0.4em? This does not seem to be correct, check with Predo.
auto-complete.js
Outdated
| const newText = | ||
| ` | ||
| <div style="margin: 0.4em" class="row"> | ||
| <span style="font-size: 10px" type="button" class="chip chip--outline chip--sm" data-toggle="tooltip" data-placement="bottom-right" |
There was a problem hiding this comment.
type="button" is not a valid attribute here, also, it does not behave as a button.
| <span style="font-size: 10px" type="button" class="chip chip--outline chip--sm" data-toggle="tooltip" data-placement="bottom-right" | |
| <span style="font-size: 10px" class="chip chip--outline chip--sm" data-toggle="tooltip" data-placement="bottom-right" |
Change in every other span.
auto-complete.js
Outdated
| } | ||
| var footerBooleanOperators = ` | ||
| <hr style="margin: 0.4em"> | ||
| <div id= "search" style="margin-left: 0.6em" > |
There was a problem hiding this comment.
Do not use id, this script will be executed more than once in SERP home for example and the id will conflict.
| <div id= "search" style="margin-left: 0.6em" > | |
| <div style="margin-left: 0.6em"> |
There was a problem hiding this comment.
Also, all sizes using em unit seems to be wrong, ask Predo about this.
auto-complete.js
Outdated
| </span> | ||
| </div> ` | ||
|
|
||
| document.getElementById('search').innerHTML = newText; |
There was a problem hiding this comment.
Avoid using id, this won't work if there is 2 or more elements in the same page with same id. Also, search is too common name to be used for id and likely to conflict.
| document.getElementById('search').innerHTML = newText; | |
| this.parentElement.innerHTML = newText; |
auto-complete.js
Outdated
| var footerBooleanOperators = ` | ||
| <hr style="margin: 0.4em"> | ||
| <div id= "search" style="margin-left: 0.6em" > | ||
| <button class="btn btn--flat btn--blue" style="font-size: 10px" onclick= "changeText()" > |
There was a problem hiding this comment.
Like I said above
| <button class="btn btn--flat btn--blue" style="font-size: 10px" onclick= "changeText()" > | |
| <button class="btn btn--flat btn--blue autocomplete-boolean-button" style="font-size: 10px"> |
auto-complete.js
Outdated
| DICAS PARA ESPECIFICAR SUA BUSCA | ||
| </button> | ||
| </div>` |
There was a problem hiding this comment.
We use 2 spaces for indentation level
| DICAS PARA ESPECIFICAR SUA BUSCA | |
| </button> | |
| </div>` | |
| DICAS PARA ESPECIFICAR SUA BUSCA | |
| </button> | |
| </div>` |
auto-complete.js
Outdated
| document.getElementById('search').innerHTML = newText; | ||
|
|
||
| } | ||
| var footerBooleanOperators = ` |
There was a problem hiding this comment.
Do not use template string, for the same reason I said above.
| .row{ | ||
| margin-inline-start: 5px; | ||
| } |
There was a problem hiding this comment.
Remove this, it's a mystique class
| font-size: 10px; | ||
| line-height: 16px; | ||
| margin-inline-start: 10px; | ||
| align-items: center; |
There was a problem hiding this comment.
This does not work without flex, remove
| border-color:black; | ||
| } | ||
|
|
||
| .autocomplete-chip::before { |
There was a problem hiding this comment.
This element does not have any text, also it should keep the mystique default style, remove
| } | ||
|
|
||
| .autocomplete-chip:hover { | ||
| border-color:black; |
There was a problem hiding this comment.
| border-color:black; | |
| border-color: black; |
| var top = pos.top + 1; | ||
| var clonedNode = this.cloneNode(true); | ||
| clonedNode.style = | ||
| 'color:transparent;box-shadow:none;z-index:10000;position:absolute;left:' + left + 'px;top:' + top + 'px'; |
There was a problem hiding this comment.
| 'color:transparent;box-shadow:none;z-index:10000;position:absolute;left:' + left + 'px;top:' + top + 'px'; | |
| 'color:transparent;box-shadow:none;z-index:10000;position:absolute;left:' + left + 'px;top:' + top + 'px'; |
| 'color:transparent;box-shadow:none;z-index:10000;position:absolute;left:' + left + 'px;top:' + top + 'px'; | ||
|
|
||
| addEvent(clonedNode, 'mouseout', function () { | ||
|
|
There was a problem hiding this comment.
Where is the remove child function?
| }) | ||
|
|
||
| function renderText(tooltip, text) { |
No description provided.