Code Review Friday: Let's Improve your code, ask Corvid master

May 29th, 2020
Join this week code review organized by Corvid master,
Comment your code below(as per instruction) we will analyze provide you suggestion or refactored code that will be easy to read, to modify, and plenty of good practice.

We also welcome experience corvid forum members to share the knowledge and expertise.

Instructions:

*1: FULL PAGE CODE is recommended
*2: Include database structure like Collection name & Field type (if you refer them in the code)
3: Is this a live site? If so, drop a link so, we can check the code.
4: Videos or Images for explanation of what the code does is useful!
Marked * are required

Don’ts

1: Do NOT include any sensitive code (like passwords, secrect key).

// try to rewrite it as xxx if it's used inside the code
const key = "XXX-XXX";

2: Asking any questions unrelated to code review or corvid
3: Screenshot the code and not pasting the code in text format
4: Code that was not working before and asking to fix (Please create a new post in the community, This Post is dedicated to improve your current working code)

Notes

  • We do code review every friday PST timezone!

  • This post will be locked after 24 hours

Hi, thanks again for this helpful initiative.

I’m trying. to replicate this example
[https://www.wix.com/corvid/forum/tips-tutorials-examples/example-repeater-dropdown-with-keyboard-navigation](https://www.wix.com/corvid/forum/tips-tutorials-examples/example-repeater-dropdown-with-keyboard-navigation

It)

It works correctly but I have the following issue:

  • if I try to select the country with the mouse, the log console shows “clicked” but does not select the country
  • once I tried with the mouse selection, it désactivante the keyboard selection and I am blocked
  • if I select “France”, it supposed to go to the URL mentioned in the database but nothing happens.

Any idea how can I select the country properly either way keyboard or mouse?

My thoughts is as follows:

  • Export function onclick is undetermined

  • Wix location API is absent from the code
    It would be very helpful if you can help me to complete the code.

Many many thanks!

Here the codes:

import wixData from ‘wix-data’ ;

const HL_COLOR = “rgba(190,190,250)” ;
const REG_COLOR = “rgba(222,222,222)” ;

let listSize;
let currIndex = - 1 ;

$w.onReady( function () {

$w( ‘#input’ ).onKeyPress((event) => {
setTimeout(() => {
if ($w( ‘#input’ ).value.length === 0 ) {
currIndex = - 1 ;
$w( “#rptDropdown” ).collapse()
.then(() => {
console.log( “Done with collapse” );
});
} else {

switch (event.key) {
case “Enter” :
$w( ‘#input’ ).value = $w( ‘#rptDropdown’ ).data[currIndex].title;
$w( “#rptDropdown” ).collapse()
.then(() => {
console.log( “Done with collapse” );
});
break ;
case “ArrowLeft” :
case “ArrowRight” :
break ;
case “ArrowUp” :
if (currIndex > 0 ) {
currIndex -= 1 ;
refresh_repeater();
}
break ;
case “ArrowDown” :
if (currIndex < listSize - 1 ) {
currIndex += 1 ;
refresh_repeater();
}
break ;
case “Escape” :
$w( ‘#input’ ).value = ‘’ ;
currIndex = - 1 ;
$w( “#rptDropdown” ).collapse()
.then(() => {
console.log( “Done with collapse” );
});
break ;
default :
currIndex = - 1 ;
wixData.query( “Countries” )
.startsWith( “title” , $w( ‘#input’ ).value)
.ascending( “title” )
.limit( 5 )
.find()
.then((res) => {
$w( ‘#rptDropdown’ ).data = ;
$w( ‘#rptDropdown’ ).data = res.items;
listSize = res.items.length;
$w( ‘#rptDropdown’ ).expand();
});
break ;
}
}
}, 50 )
});
});

export function rptDropdown_itemReady($item, itemData, index) {
$item( ‘#dataset1’ ).text = itemData.title;

if (index === currIndex) {
$item( “#rptBox” ).style.backgroundColor = HL_COLOR;
} else {
$item( “#rptBox” ).style.backgroundColor = REG_COLOR;
}

$item( '#container1' ).onClick(() => { 
    $w( '#input' ).value = itemData.title; 
    $w( '#rptDropdown' ).collapse(); 
}); 

}

function refresh_repeater() {
$w( “#rptDropdown” ).forEachItem(($item, itemData, index) => {
$item( ‘#dataset1’ ).text = itemData.title;

if (index === currIndex) {
$item( “#rptBox” ).style.backgroundColor = HL_COLOR;
} else {
$item( “#rptBox” ).style.backgroundColor = REG_COLOR;
}

    $item( '#container2' ).onClick(() => { 
        $w( '#input' ).value = itemData.title; 
        $w( '#rptDropdown' ).collapse(); 
    }); 
}); 

}

export function name_click(event) {
//Add your code for this event here:
console.log( ‘clicked’ )

}

I appreciate your interest but this section is to rewrite working code for better readability and scalability.
This code was written by Yisrael and i don’t think it need to be refactor.
regarding the question, i see you already post this question on Yisrael post
Create a new post and tag me there

My question today is different from the previous asked to Yisrael. I don’t get the necessary to create a new post. I think the forum is to help each other and not to work especially on admin stuff.
I also do not ask to refactor Yisrael code.
Clearly, I miss your point here

Comment your code below(as per instruction) we will analyze provide you suggestion or refactored code that will be easy to read, to modify, and plenty of good practice.

Read the 4th point of Don’t
This post is dedicated to refactor existing code.
that’s why i asked you to create a new post

improving your existing code is different than adding a new capabilities/ bug fix to the code

(I will remove your thread later. just to make things organize.)

i am willing to help you, just create a new post so, it’s organized and other can benefit from it and this post will be locked in 24 hours.

Hello,

Thanks a lot for the initiative, this is a great idea ! :slight_smile:

So, here’s the thing : I am trying to call different items of my database in order to have one dynamic page in French and in Spanish displaying different descriptions. The code works fine as it indeed calls two different descriptions for text1, but I have 4 rows in my dataset and on the 4 pages, only the text of the last row appears. Can’t get around the reason why…
Any help would be highly appreciated !

Thank you in advance !

Here’s the code :

import wixData from 'wix-data';

import wixWindow from 'wix-window';

$w.onReady(function () {

$w("#dynamicDataset").onReady( () => {

 console.log("the dataset is ready");

 update_items();

 let itemObj = $w("#dynamicDataset").getCurrentItem();

$w("#dynamicDataset").onCurrentIndexChanged( (index) => {

 console.log("the item changed");

 update_items();

});

});

function update_items () {

const current_item = $w("#dynamicDataset").getCurrentItemIndex();

let language = wixWindow.multilingual.currentLanguage; 

let description_text;

console.log(current_item);

wixData.query("PRODUITS").find().

then( (results) => {  

 if(language === "es"){

  description_text = results.items[current_item].description_es;

 } else if (language === "fr") {

  description_text = results.items[current_item].description_fr;

 } else {

  description_text = results.items[current_item].description_fr;

 }

 $w("#text1").text = description_text;

}); 

}
});

This is a god awful mash up but I don’t know how to improve it from here… any pointers would be welcome

I have a repeater with items in it, and two filters applied:

  1. Tag filtering, which filters by repopulating the repeater

  2. Input field filtering, which filters by setFilter

So the tag filter works great, but I can’t get the input field to behave - it’ll randomly show all, or I have to type and retype… eventually the right items will show up if I type and delete enough, but I want them to work together and not be a janky hack.

Page is here: https://www.antscihub .com/caresheets

The collection name is Caresheets
The field the tags look for and filter is caresheetTags
The tag box is #tag
The repeater is #csRepeater
The search box is #iLocation
The dataset is #csdb
The field the input box is searching is overviewRange

I’m not sure if this is within the scope of this post, so please let me know if it is not. Thank you for taking a look!

// For full API documentation, including code examples, visit https://wix.to/94BuAAs
import wixData from 'wix-data';

const collectionName = 'Caresheets';
const fieldToFilterByInCollection = 'caresheetTags';



$w.onReady(function () {
	//TODO: write your page related code here...
setRepeatedItemsInRepeater()
	loadDataToRepeater()

	$w('#tags').onChange((event) => {
		const selectedTags = $w('#tags').value
		loadDataToRepeater(selectedTags)
	})
});

function loadDataToRepeater(selectedCategories = []) {

	let dataQuery = wixData.query(collectionName)

	if (selectedCategories.length > 0) {
		dataQuery = dataQuery.hasAll(fieldToFilterByInCollection, selectedCategories)
	}
	
	dataQuery
		.find()
		.then(results => {
			const itemsReadyForRepeater = results.items
			$w('#csRepeater').data = itemsReadyForRepeater;

			const isRepeaterEmpty = itemsReadyForRepeater.length === 0

			if (isRepeaterEmpty) {
		//		$w('#noResultsFound').show()
			} else {
		//		$w('#noResultsFound').hide()
			}
		})
}

let debounceTimer;
export function iLocation_keyPress_1(event, $w) {
		console.log($w('#iLocation').value);
		filter($w('#iLocation').value);
				if (debounceTimer) {
			clearTimeout(debounceTimer);
			debounceTimer = undefined;
		}
		debounceTimer = setTimeout(() => {
			filter($w('#iLocation').value);
		}, 200);
	}	


let lastFilterLocation
function filter(range) {
	if (lastFilterLocation !== range) {
		$w('#csdb').setFilter(wixData.filter().contains('overviewRange', range));
		lastFilterLocation = range;
	}
}

function setRepeatedItemsInRepeater() {
	$w('#csRepeater').onItemReady(($item, itemData) => {

		$item('#csRepeaterSpecies').text = itemData.species;
	})
}

Hi, Juwairia here again. I am building a job board in wix that would display jobs from a database, with each job in its own repeater. My relevant database fields are: PositionTitle, CompanyName, Industry, Description,

On this page, I have a search that searches through PositionTitle & CompanyName and a dropdown filter that sorts through position type.

I’d like for there to be a ‘no results found’ text box come up when no results are found from a users text input in the search box. Secondly, I am trying to incorporate a clear filter button for users to clear the filter theyve applied and revert the listings to their original display.

Currently, the clear filter button kind of works, but when clearing it displays the top-most option in the dropdown, but the repeaters are shown in the original order. So my dropdown is organized by Full-time, part-time, intern, etc, so when I hit clear search, the dropdown displays full-time. This is a slight lag, but may confuse users. Is there any way this can be fixed?

exportfunction button10_click(event) {//Add your code for this event here:     $w("#dropdown").selectedIndex = 0    $w("#dynamicDataset").setFilter(wixData.filter());}

To display the ‘0 results found’ i tried including a results counter under the search code, but its just counting everything in my database. How can I edit it to show only the results found - which should be 0? Or any other way to show no results found is fine too.

$w.onReady(function () {let count = $w("#dynamicDataset").getTotalCount();    $w("#counter").text = String(count) + " jobs posted";})

Full code below. It also includes code for a job counter & a show more toggle in each repeater.

import wixData from "wix-data";

$w.onReady(function () {
    $w("#dynamicDataset").onReady(() => {
 // SHORT DESC CODE
        $w("#listRepeater").onItemReady(($item, itemData) => {
 const { description } = itemData;

 // on item ready set the desc shorter
            $item("#desc").text = shortText(description);
            $item("#showmore").label = "Show More";

            $item("#showmore").onClick((event) => {
 // toggle short desc.
 if ($item("#showmore").label === "Show More") {
                    $item("#desc").text = description;
                    $item("#showmore").label = "Show Less";
                } else {
                    $item("#desc").text = shortText(description);
                    $item("#showmore").label = "Show More";
                }
            });
        });

 function shortText(text, maxLen = 220) {
 return text.substr(0, maxLen) + "...";
        }
    });

 //  FILTER CODE
    $w("#searc").onKeyPress(() => {
 // keypress event trigger before the dom is rendered
 // setTimeout will wait 40ms before excuting the code in filterDs
        setTimeout(filterDs, 40);
    });
 // Dropdown with all the company name
    $w("#dropdown").onChange(() => {
        filterDs();
    });
});

function filterDs() {
 // inital filter
 let filterBuilder = wixData.filter();

 let keyword = $w("#searc").value;
 let dropdown = $w("#dropdown").value;

 if (keyword) {
        filterBuilder = filterBuilder.contains("positionTitle", keyword)
            .or(wixData.filter().contains("companyName", keyword));
 //lastFilterSearchBox = searchBox;
    }

 if (dropdown) {
        filterBuilder = filterBuilder.eq("jobType", dropdown);
        console.log("Dropdown value: " + dropdown);
    }
    $w("#dynamicDataset").setFilter(filterBuilder);

}

$w.onReady(function () {
 let count = $w("#dynamicDataset").getTotalCount();
    $w("#counter").text = String(count) + " jobs posted";
})

export function button10_click(event) {
 //Add your code for this event here: 
    $w("#dropdown").selectedIndex = 0
    $w("#dynamicDataset").setFilter(wixData.filter());
}


 
  • Hey, you can simplify this by using repeater onItemReady*

  • Which will run for every container with two arguments*

  • $item => simillar $w but this selector select one of the repeater element $item(“text1”), if the text1 is a element in the repeater*

  • onItemReady function will loop for every container in that repeater*

  • and you can use $item to select for the specific container*

  • itemData => have the current Item data for that container*


import wixWindow from 'wix-window';

const lang = wixWindow.multilingual.currentLanguage;

$w.onReady(function () {

  $w("#dynamicDataset").onReady(() => {

    $w('#repeater').onItemReady(($item, itemData) => {
 let description = itemData[lang === "es" ? "description_es" : "description_fr"];
      console.log({ description, itemData });
      $item('#text1').text = description;
    });

  });
});

Thanks for the live link and collection and fieldkey details!
you can remove comments start with “comment :”, just to explain
why we did it.

This is exactly with in the scope :slight_smile:

import wixData from 'wix-data';

$w.onReady(function () {
  filterRepeater();

 // event
 // comment : onInput is better than onKeypress it will 
 // comment : run after the key is register, so we don't need to use a delay
 // comment : but we do need a debouncer so, we can limit the function call

 let debouncer;
  $w('#iLocation').onInput(()=>{
 if( debouncer ) return ;
    debouncer = setTimeout(()=>{
      debouncer = undefined;
      filterRepeater();
    }, 200);
  });


  $w('#tags').onChange((event) => {
    filterRepeater();
  });

 // comment : you don't need to use a function to call
 // comment : everytime when the data change, you can just put it on page ready
  $w('#csRepeater').onItemReady(($item, itemData) => {
    $item('#csRepeaterSpecies').text = itemData.species;
  })
});



async function filterRepeater() {
 const location = $w('#iLocation').value;
 const categories = $w('#tags').value;

 if(location)

 let filter = wixData.filter();

 if( location.length > 0 ) {
 // comment : onInput runs only on input change (i think) unlike keypress
 // comment : where we need to check if the input value is change
 // comment : using the lastFilterLocation
    filter = filter.contains('overviewRange', location);
  }
 
 if ( categories.length > 0 ) {
    filter = filter.hasAll("caresheetTags", categories)
  }
 
  console.log("filter object : " , filter);
 await $w('#csdb').setFilter(filter);

 let ttl = $w('#csdb').getTotalCount();
  console.log("total item found : " , ttl);

 if( ttl > 0 ) {
 // $w('#noResultsFound').show()
  } else {
 // $w('#noResultsFound').hide()
  }
}


You were almost there



import wixData from "wix-data";

$w.onReady(function () {
  $w("#dynamicDataset").onReady(() => {
 // SHORT DESC CODE
    $w("#listRepeater").onItemReady(($item, itemData) => {
 const { description } = itemData;

 // on item ready set the desc shorter
      $item("#desc").text = shortText(description);
      $item("#showmore").label = "Show More";

      $item("#showmore").onClick((event) => {
 // toggle short desc.
 if ($item("#showmore").label === "Show More") {
          $item("#desc").text = description;
          $item("#showmore").label = "Show Less";
        } else {
          $item("#desc").text = shortText(description);
          $item("#showmore").label = "Show More";
        }
      });
    });

 function shortText(text, maxLen = 220) {
 return text.substr(0, maxLen) + "...";
    }

 // comment : change button10 to btnClear => stands for "button Clear" 
    $w('#button10').onClick(()=>{
      $w("#dropdown").value = "";
      $w("#dynamicDataset").setFilter(wixData.filter());
    })

  });

 //  FILTER CODE
  $w("#searc").onKeyPress(() => {
 // keypress event trigger before the dom is rendered
 // setTimeout will wait 40ms before excuting the code in filterDs
    setTimeout(filterDs, 40);
  });
 // Dropdown with all the company name
  $w("#dropdown").onChange(() => {
    filterDs();
  });
});

async function filterDs() {
 // inital filter
 let filterBuilder = wixData.filter();

 let keyword = $w("#searc").value;
 let dropdown = $w("#dropdown").value;

 if (keyword) {
    filterBuilder = filterBuilder.contains("positionTitle", keyword)
      .or(wixData.filter().contains("companyName", keyword));
 //lastFilterSearchBox = searchBox;
  }

 if (dropdown) {
    filterBuilder = filterBuilder.eq("jobType", dropdown);
    console.log("Dropdown value: " + dropdown);
  }
  await $w("#dynamicDataset").setFilter(filterBuilder);

 
 let count = $w("#dynamicDataset").getTotalCount();
  $w("#counter").text = String(count) + " jobs posted";

}


Closing the thread
hope this was useful,
We(Corvid master) will do Code review on every Friday ,
get ready for the next code review on June 5th 2020
Happy coding :v: