Code Review Friday: (Let's Improve your code, ask Corvid Master)

Ask the community
, ,
1

July 10th, 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 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 an explanation of what the code does is useful!
Marked * are required

Don’ts

1: Do NOT include any sensitive code (like passwords, secret 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)

Comment not related to refactoring for the code will be get deleted to organize

Notes

  • We do code review every Friday PST timezone!

  • This post will be locked after 24 hours

2

Hello again :grin::sweat_smile:,

like every friday i have some work for you xD.

Ok let’s go…

i have written a code to place specific data to elements given in a repeater.
My question is, if there is a better way to code, then the way i did?

import wixData from 'wix-data';

 var ItemLength


  $w.onReady(function () {
    wixData.query("Collection")
    .find()
    .then( (res) => {
 let Item = res.items
      ItemLength = res.items.length
 
      wixData.query("Products")
      .find()
      .then( (results) => {
 let item = results.items
 
        $w("#repeater1").onItemReady( ($item, itemData, index) => {
 for (var i = ItemLength-1; i >= 0; i=i-1) {console.log("i = " + i), console.log("index = " + index)
          console.log('--------------------')
 if (itemData.title === Item[i].title) {$item("#TXTtype1").text = results.items[(index)*5+i+0-(1*index)].type, $item("#IMGtype1").src = results.items[(index)*5+i+0-(1*index)].image, console.log("Output-Index1 = " + index)}
 if (itemData.title === Item[i].title) {$item("#TXTtype2").text = results.items[(index)*5+i+1-(1*index)].type, $item("#IMGtype2").src = results.items[(index)*5+i+1-(1*index)].image, console.log("Output-Index2 = " + index)}
 if (itemData.title === Item[i].title) {$item("#TXTtype3").text = results.items[(index)*5+i+2-(1*index)].type, $item("#IMGtype3").src = results.items[(index)*5+i+2-(1*index)].image, console.log("Output-Index3 = " + index)}
 if (itemData.title === Item[i].title) {$item("#TXTtype4").text = results.items[(index)*5+i+3-(1*index)].type, $item("#IMGtype4").src = results.items[(index)*5+i+3-(1*index)].image, console.log("Output-Index4 = " + index)}
 if (itemData.title === Item[i].title) {$item("#TXTtype5").text = results.items[(index)*5+i+4-(1*index)].type, $item("#IMGtype5").src = results.items[(index)*5+i+4-(1*index)].image, console.log("Output-Index5 = " + index)}
          console.log('--------------------')
        }
      });
    });
  });
});

Here is this example…
https://russian-dima.wixsite.com/meinewebsite/blank-16

And here the related databases…

Collection…

764×634 42.3 KB

Products…

1756×1474 99 KB

And an additional question.
I know that this function is not very flexible, because the code allows just 5-product-items per 1-collection-item. So my question would be how to do it more flexible, so that it would not matter if there are 4,5,6 or more items per collection given in the product-database? Is this possible?

And i say right now already BIG-THANKS, because this POSTS closes and you have not always the time to say it :wink:

3

Hello, I have a collection with a 2031 rows and actually I can do data queries up to the 1st on thousand rows. How can I find a solution to query the full number of rows? Here my codes:

function filtreCom() {
wixData.query( “MobileforfaitFR” ).ascending( “call” )
.limit( 1000 )
.find()
.then(results => {
const uniqueItems = choixUnique(results.items);
let single = option(uniqueItems);
single.unshift({ “label” : “All” , “value” : “All” });
$w( “#dropCom” ).options = single;
});

function choixUnique(items) {
const itemsOnly = items.map(item => item.call);
return [… new Set(itemsOnly)];
}

function option(uniqueList) {
return uniqueList.map(curr => {
return { label: curr, value: curr };
});
}
}

4

/*
It’s better to use a reference field, to connect the Collection databas with the product database
Once you create a column and set the data type to reference in the Product database and point to Collection database
Then you can use database UI to search for the collection and select the collection for each product

For now we can’t use repeater inside of repeater, So, your’s is the only solution to use a fixed Number elements
in your case it’s five

But you can attach the table inside of the repater to display multiple prooduct
If you done it right you don’t need any code to connect the table to the database

*/

import wixData from ‘wix-data’ ;

var ItemLength

$w . onReady ( function () {
wixData . query ( “Collection” )
. find ()
. then (( res ) => {
let Item = res . items
ItemLength = res . items . length

wixData . query ( “Products” )
. find ()
. then (( results ) => {
let product = results . items ;

$w ( “#repeater1” ). onItemReady (( $item , itemData , index ) => {
for ( var i = ItemLength - 1 ; i >= 0 ; i = i - 1 ) {
const currTitle = Item [ i ]. title ;
const iteration = ( index * 4 ) + i ;
console . log ( "i = " + i ); console . log ( "index = " + index );
console . log ( ‘--------------------’ );

if ( itemData . title === currTitle ) {
$item ( “#TXTtype1” ). text = product [ iteration ]. type ;
$item ( “#IMGtype1” ). src = product [ iteration ]. image ;
console . log ( "Output-Index1 = " + index )

$item ( “#TXTtype2” ). text = product [ iteration + 1 ]. type ;
$item ( “#IMGtype2” ). src = product [ iteration + 1 ]. image ;
console . log ( "Output-Index2 = " + index )

$item ( “#TXTtype3” ). text = product [ iteration + 2 ]. type ;
$item ( “#IMGtype3” ). src = product [ iteration + 2 ]. image ;
console . log ( "Output-Index3 = " + index )

$item ( “#TXTtype4” ). text = product [ iteration + 3 ]. type ;
$item ( “#IMGtype4” ). src = product [ iteration + 3 ]. image ;
console . log ( "Output-Index4 = " + index )

$item ( “#TXTtype5” ). text = product [ iteration + 4 ]. type ;
$item ( “#IMGtype5” ). src = product [ iteration + 4 ]. image ;
console . log ( "Output-Index5 = " + index )
}
console . log ( ‘--------------------’ )
}
});
});
});
});

5

/*
It’s better to use distinct, Which will give you an array of unique items.
you don’t need to loop through 1000 items and get unique item
There is not limitation specific in the documentation
https://www.wix.com/corvid/new-reference/wix-data/wixdataquery/distinct

so i think it should work if you have less than 1000 unique items
like if the database has 10k rows but only 600 unique calls values
than it should work
I assume the limitation up to 1000 unique item, there might be no limitation

*/

function filtreCom() {
wixData.query( “MobileforfaitFR” )
.ascending( “call” )
.distinct( “call” )
.then(res => {
const calls = res.items;
let opts = toOptions(calls);
opts.unshift({ “label” : “All” , “value” : “All” });
$w( “#dropCom” ).options = opts;
});

function toOptions(arr=) {
let toLabel = curr => ({ label: curr, value: curr });
return arr.map(toLabel);
}
}

6

hi, I perform a logout() via code, following the log out there is a redirection to the the home page " wixLocation.to( “/” )", but never happen always stay in current page. this is my full Site Code:


import wixWindow from 'wix-window';
import wixUsers from 'wix-users';
import { session } from 'wix-storage';
import wixLocation from 'wix-location';
import wixData from 'wix-data';

let previousPageURL;

$w.onReady(function () {

    previousPageURL = session.getItem("page");
    console.log(previousPageURL);
    session.setItem("page", wixLocation.url);
    $w("#button14").link = previousPageURL;
    $w("#button14").target = "_self";
    $w('#home').onClick(() => {
        wixWindow.openLightbox("test")

    })

 if (wixUsers.currentUser.loggedIn) {
        $w("#logButton").label = "Logout";
        $w("#myprofbutton").show();
    } else {
        $w("#logButton").label = "Login";
        $w("#myprofbutton").hide();
    }
});

export function logButton_click(event) {

 if (wixUsers.currentUser.loggedIn) {
 // log the user out
        wixUsers.logout()
            .then(() => {
 // update buttons accordingly
                wixLocation.to("/")
                $w("#logButton").label = "Log In";
                $w("#myprofbutton").hide();

            });
    } else {
 let userId;
 let userEmail;
 // prompt the user to log in 
        wixWindow.openLightbox("Login")
            .then((user) => {
                userId = user.id;
 return user.getEmail();
            })
            .then((email) => {
 // check if there is an item for the user in the collection
                userEmail = email;
 return wixData.query("Members")
                    .eq("_id", userId)
                    .find();
            })
            .then((results) => {
 // if an item for the user is not found
 if (results.items.length === 0) {
 // create an item
 const toInsert = {
 "_id": userId,
 "email": userEmail
                    };
 // add the item to the collection
                    wixData.insert("Members", toInsert)
                        .catch((err) => {
                            console.log(err);
                        });
                }
 // update buttons accordingly
                $w("#logButton").label = "Log Out";
                $w("#myprofbutton").show();
            })
            .catch((err) => {
                console.log(err);
            });
    }
}

export function myprofbutton_click(event) {
    wixLocation.to(`/members/${wixUsers.currentUser.id}`);
}

thanks.

7

Try this
without using .then
if (wixUsers.currentUser.loggedIn) {
wixUsers.logout()
wixLocation.to( “/” )
}

8

Thanks a lot like every Friday for very good informations :grin::wink:

9

Happy to help and thank you for the support :slight_smile:

10

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 17th July
Happy coding :v: