jQuery âon('click')â to toggle webpage content
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
2
down vote
favorite
I have made a successful test page that satisfies my goals of using JavaScript ( jQuery specifically ) to add and remove content from a webpage.
<!doctype html>
<head>
<title>test</title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
</head>
<body>
<div id="main"></div>
<script>
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
$("#main").append(foo);
$("#main").on('click','h3#bar_link', function ()
console.log("Clicked bar_link");
$("#main").html("");
$("#main").append(bar);
).on('click','h3#foo_link', function ()
console.log("Clicked foo_link");
$("#main").html("");
$("#main").append(foo);
).on('click','h1#title', function ()
console.log("Clicked title");
$("#main").html("");
$("#main").append(foo);
)
</script>
</body>
</html>
For this simple example where only two possible outcomes exist the code isn't too bad to look at, however I know that if this were inflated to what a full website can entail (e.g. dozens of links per page, each one putting in different content) the .on(click)
chain would get very complicated.
How else could I handle this situation?
I know that I could just make two different .html files ( foo and bar ) and setup simple anchor links (i.e. <a href="...">
) between them and obtain the same effect but my goal is to have an external source generate the content that is displayed.
I am getting used to JavaScript programming in general so please do point out other ways I can improve what I am doing.
javascript beginner jquery html
add a comment |Â
up vote
2
down vote
favorite
I have made a successful test page that satisfies my goals of using JavaScript ( jQuery specifically ) to add and remove content from a webpage.
<!doctype html>
<head>
<title>test</title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
</head>
<body>
<div id="main"></div>
<script>
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
$("#main").append(foo);
$("#main").on('click','h3#bar_link', function ()
console.log("Clicked bar_link");
$("#main").html("");
$("#main").append(bar);
).on('click','h3#foo_link', function ()
console.log("Clicked foo_link");
$("#main").html("");
$("#main").append(foo);
).on('click','h1#title', function ()
console.log("Clicked title");
$("#main").html("");
$("#main").append(foo);
)
</script>
</body>
</html>
For this simple example where only two possible outcomes exist the code isn't too bad to look at, however I know that if this were inflated to what a full website can entail (e.g. dozens of links per page, each one putting in different content) the .on(click)
chain would get very complicated.
How else could I handle this situation?
I know that I could just make two different .html files ( foo and bar ) and setup simple anchor links (i.e. <a href="...">
) between them and obtain the same effect but my goal is to have an external source generate the content that is displayed.
I am getting used to JavaScript programming in general so please do point out other ways I can improve what I am doing.
javascript beginner jquery html
add a comment |Â
up vote
2
down vote
favorite
up vote
2
down vote
favorite
I have made a successful test page that satisfies my goals of using JavaScript ( jQuery specifically ) to add and remove content from a webpage.
<!doctype html>
<head>
<title>test</title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
</head>
<body>
<div id="main"></div>
<script>
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
$("#main").append(foo);
$("#main").on('click','h3#bar_link', function ()
console.log("Clicked bar_link");
$("#main").html("");
$("#main").append(bar);
).on('click','h3#foo_link', function ()
console.log("Clicked foo_link");
$("#main").html("");
$("#main").append(foo);
).on('click','h1#title', function ()
console.log("Clicked title");
$("#main").html("");
$("#main").append(foo);
)
</script>
</body>
</html>
For this simple example where only two possible outcomes exist the code isn't too bad to look at, however I know that if this were inflated to what a full website can entail (e.g. dozens of links per page, each one putting in different content) the .on(click)
chain would get very complicated.
How else could I handle this situation?
I know that I could just make two different .html files ( foo and bar ) and setup simple anchor links (i.e. <a href="...">
) between them and obtain the same effect but my goal is to have an external source generate the content that is displayed.
I am getting used to JavaScript programming in general so please do point out other ways I can improve what I am doing.
javascript beginner jquery html
I have made a successful test page that satisfies my goals of using JavaScript ( jQuery specifically ) to add and remove content from a webpage.
<!doctype html>
<head>
<title>test</title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
</head>
<body>
<div id="main"></div>
<script>
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
$("#main").append(foo);
$("#main").on('click','h3#bar_link', function ()
console.log("Clicked bar_link");
$("#main").html("");
$("#main").append(bar);
).on('click','h3#foo_link', function ()
console.log("Clicked foo_link");
$("#main").html("");
$("#main").append(foo);
).on('click','h1#title', function ()
console.log("Clicked title");
$("#main").html("");
$("#main").append(foo);
)
</script>
</body>
</html>
For this simple example where only two possible outcomes exist the code isn't too bad to look at, however I know that if this were inflated to what a full website can entail (e.g. dozens of links per page, each one putting in different content) the .on(click)
chain would get very complicated.
How else could I handle this situation?
I know that I could just make two different .html files ( foo and bar ) and setup simple anchor links (i.e. <a href="...">
) between them and obtain the same effect but my goal is to have an external source generate the content that is displayed.
I am getting used to JavaScript programming in general so please do point out other ways I can improve what I am doing.
<!doctype html>
<head>
<title>test</title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
</head>
<body>
<div id="main"></div>
<script>
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
$("#main").append(foo);
$("#main").on('click','h3#bar_link', function ()
console.log("Clicked bar_link");
$("#main").html("");
$("#main").append(bar);
).on('click','h3#foo_link', function ()
console.log("Clicked foo_link");
$("#main").html("");
$("#main").append(foo);
).on('click','h1#title', function ()
console.log("Clicked title");
$("#main").html("");
$("#main").append(foo);
)
</script>
</body>
</html>
<!doctype html>
<head>
<title>test</title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
</head>
<body>
<div id="main"></div>
<script>
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
$("#main").append(foo);
$("#main").on('click','h3#bar_link', function ()
console.log("Clicked bar_link");
$("#main").html("");
$("#main").append(bar);
).on('click','h3#foo_link', function ()
console.log("Clicked foo_link");
$("#main").html("");
$("#main").append(foo);
).on('click','h1#title', function ()
console.log("Clicked title");
$("#main").html("");
$("#main").append(foo);
)
</script>
</body>
</html>
javascript beginner jquery html
edited Feb 7 at 16:50
Sam Onela
5,88461545
5,88461545
asked Feb 6 at 15:13
Reizvoller
333
333
add a comment |Â
add a comment |Â
3 Answers
3
active
oldest
votes
up vote
1
down vote
accepted
Well if you use event delegation
on the <h3>
element you won't have to care for how many pages or links there are just bind it once on page load and it will take care of the rest assuming the links are based on h3
following your given example so that solves your main problem.
$(document).on('click', 'h3', function()
//do your thing
);
Since you have stated that you have an external source to generate the HTML I would create a function pageloader()
to load the content.if that is going to be an ajax function to get the content or a template file using mutache.js
or any other.
function pageLoader()
$.ajax(
url:'path/to/content/',
//other params...
success:function(data)
//load content here
$("#map").html(data);
,
);
But if that content was to be used in the same scenario as you have provided
then you have the option of creating object literals
for keeping all the pages in one place and calling them from one place and when calling use hasOwnProperty()
to avoid inherited Properties.
var pages=
pagename:function()
//load the content of the page
,
pagename2:function()
//load content of the page
In this case What you need to consider is that you either keep the id
of the h3
similar to the page name or define a data-attribute on the h3 and add the page name there, as we would be passing that id
/data-attribute
as a parameter to our pageLoader()
function which holds all our pages and calls them too why because we will create the properties of the object literal as page name and then load them.
Below is a demonstration although I cant create an ajax example to show what I talked about in the first section, i will use the object literal section to create a demo since your actual concern was the binding of the links and not how to load the content, and that is covered. So you can update the pageLoader()
function with whichever approach you want to load the content
function pageLoader(pagename)
"use strict";
var pages =
"foo": function()
$("#main").html(`<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"bar": function()
$("#main").html(`<h1 id="title">Page-Bar</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"contact": function()
$("#main").html(`<h1 id="title">Page-Contact</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"about": function()
$("#main").html(`<h1 id="title">Page-about-Two</h1></div>
<p>Some text about about about about.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
;
//call the page
if (pages.hasOwnProperty(pagename))
pages[pagename].call(this);
//call on page load for the first time
$(document).ready(function()
pageLoader("foo");
);
//use event delegation and bind the h3
$(document).on('click', 'h3', function()
let pagename = $(this).prop("id");
pageLoader(pagename);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="main"></div>
Hope this helps
add a comment |Â
up vote
2
down vote
The code pattern:
$("#main").html("");
$("#main").append(bar);
can be simplified to:
$("#main").html(bar);
In general, the code is, clear, easy to read, and well organized. All the decisions among alternatives are reasonable and straight forward. Doing the simplest thing that might work is a good approach because it avoids adding unneeded complexity to the necessary complexity.
The use of template literals rather than strings might be unnecessary complexity because the templating features are not used. Support for template literals is not universal among browsers. The page may break for no reason for some people. To put it another way, using template literals rather than strings creates potential broken pages for no gain in browsers that support template literals.
yes, I agree on the template literals I literally didn't notice those were backticks when copied from the OP, although they are clearlymultiline
and should have been spotted, but those backticks can be replaced withdouble
orsingle
quotes and then useMustache.js
library to render the template along with the variables/placeholder to parse. But thanks for the heads up , i appreciate.
â Muhammad Omer Aslam
Feb 7 at 17:24
add a comment |Â
up vote
1
down vote
Ben is correct- the code is mostly clear and easy to read, though there is some redundancy (e.g. $("#main").append(foo);
, and even just $("#main")
has many occurences).
I agree with Muhammad that Event delegation is a great way to simplify the logic.
Another aspect to consider is that every time $()
appears with a CSS selector (e.g. $("#main")
) a DOM look-up is performed. This can be optimized by storing that DOM reference in a variable and then substituting that variable throughout the rest of the code. For more on this topic and other optimizations, see the section Cache DOM Lookups on this post. I know that post begins by bashing jQuery but if your goal is to use it then by all means keep it. However, you might consider alternatives - see youmightnotneedjquery.com.
So you could define a constant:
const mainElement = $("#main");
And then use that everywhere else:
mainElement.append(foo);
mainElement.on('click','h3#bar_link', function ()
mainElement.html(bar); //take advice from Ben
);//... and so on ...
Additionally, the shortcut .click()
could be used in place of .on('click', handler)
. Then to use event delegation, check the attributes of the target of the event. For example, the click handler will receive the Event object, which has the target property. The id property of that target can be fetched using .attr('id')
and that could be used to determine which element was clicked and thus which action to take. Or .is()
could be used to determine if the target matches a CSS selector.
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
const mainElement = $("#main");
mainElement.append(foo);
mainElement.click(function(clickEvent)
const clickedId = $(clickEvent.target).attr('id');
if (clickedId)
if (clickedId == 'bar_link')
mainElement.html(bar);
else
mainElement.html(foo);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<div id="main"></div>
I do like how Muhammad laid out the pages
object in pageLoader()
to map id selectors to HTML strings. However, the function calls could be removed from the mapping, and move that call to .html()
to a single call later in pageLoader()
, like the snippet below:
var pages =
"foo": `<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`,
...
;
if (pages.hasOwnProperty(pagename))
$('#main').html(pages[pagename]);
That way there is only one place .html()
is called instead of once per each property of the object.
1
i definitely missed these 2 things you mentioned above all moving the$('#main').html()
outside is way better approach than the way i used. thanks for the heads-up. it was the first ever review i submitted so am all ears to suggestions.
â Muhammad Omer Aslam
Feb 7 at 17:13
I have been on StackOverflow for quite some time but as I told you that this was my first review so wanted to ask that if I could update the answer according to the suggestions by referring you and Ben both about the points you raised and change the code or I cannot? we can do that on StackOverflow but don't know about code review if it is allowed or not ?
â Muhammad Omer Aslam
Feb 7 at 17:28
Do you mean you want to update (the code in) your answer? If so, there should be a link below it on the left side (see this)... otherwise if you mean the OPs code then know that it should not be updated, lest our answers be invalidated
â Sam Onela
Feb 7 at 17:33
1
Haha obviously not OP's code at least I am used to StackOverflow rules that much:D
, i wanted to just know if i can update the code in my own post according to the suggestions you both gave me, thanks for informing.
â Muhammad Omer Aslam
Feb 7 at 17:37
just one thing more rather than removing the function call from the object literals should'nt it be better to keep it that way, because if I replace the backtick with a single quote and use theMustach.js
to render my templates and then I would have to return the HTML after rendering, because i think @Ben has a reasonable reason to replace the backticks with single or double quotes
â Muhammad Omer Aslam
Feb 7 at 17:53
 |Â
show 2 more comments
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
accepted
Well if you use event delegation
on the <h3>
element you won't have to care for how many pages or links there are just bind it once on page load and it will take care of the rest assuming the links are based on h3
following your given example so that solves your main problem.
$(document).on('click', 'h3', function()
//do your thing
);
Since you have stated that you have an external source to generate the HTML I would create a function pageloader()
to load the content.if that is going to be an ajax function to get the content or a template file using mutache.js
or any other.
function pageLoader()
$.ajax(
url:'path/to/content/',
//other params...
success:function(data)
//load content here
$("#map").html(data);
,
);
But if that content was to be used in the same scenario as you have provided
then you have the option of creating object literals
for keeping all the pages in one place and calling them from one place and when calling use hasOwnProperty()
to avoid inherited Properties.
var pages=
pagename:function()
//load the content of the page
,
pagename2:function()
//load content of the page
In this case What you need to consider is that you either keep the id
of the h3
similar to the page name or define a data-attribute on the h3 and add the page name there, as we would be passing that id
/data-attribute
as a parameter to our pageLoader()
function which holds all our pages and calls them too why because we will create the properties of the object literal as page name and then load them.
Below is a demonstration although I cant create an ajax example to show what I talked about in the first section, i will use the object literal section to create a demo since your actual concern was the binding of the links and not how to load the content, and that is covered. So you can update the pageLoader()
function with whichever approach you want to load the content
function pageLoader(pagename)
"use strict";
var pages =
"foo": function()
$("#main").html(`<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"bar": function()
$("#main").html(`<h1 id="title">Page-Bar</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"contact": function()
$("#main").html(`<h1 id="title">Page-Contact</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"about": function()
$("#main").html(`<h1 id="title">Page-about-Two</h1></div>
<p>Some text about about about about.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
;
//call the page
if (pages.hasOwnProperty(pagename))
pages[pagename].call(this);
//call on page load for the first time
$(document).ready(function()
pageLoader("foo");
);
//use event delegation and bind the h3
$(document).on('click', 'h3', function()
let pagename = $(this).prop("id");
pageLoader(pagename);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="main"></div>
Hope this helps
add a comment |Â
up vote
1
down vote
accepted
Well if you use event delegation
on the <h3>
element you won't have to care for how many pages or links there are just bind it once on page load and it will take care of the rest assuming the links are based on h3
following your given example so that solves your main problem.
$(document).on('click', 'h3', function()
//do your thing
);
Since you have stated that you have an external source to generate the HTML I would create a function pageloader()
to load the content.if that is going to be an ajax function to get the content or a template file using mutache.js
or any other.
function pageLoader()
$.ajax(
url:'path/to/content/',
//other params...
success:function(data)
//load content here
$("#map").html(data);
,
);
But if that content was to be used in the same scenario as you have provided
then you have the option of creating object literals
for keeping all the pages in one place and calling them from one place and when calling use hasOwnProperty()
to avoid inherited Properties.
var pages=
pagename:function()
//load the content of the page
,
pagename2:function()
//load content of the page
In this case What you need to consider is that you either keep the id
of the h3
similar to the page name or define a data-attribute on the h3 and add the page name there, as we would be passing that id
/data-attribute
as a parameter to our pageLoader()
function which holds all our pages and calls them too why because we will create the properties of the object literal as page name and then load them.
Below is a demonstration although I cant create an ajax example to show what I talked about in the first section, i will use the object literal section to create a demo since your actual concern was the binding of the links and not how to load the content, and that is covered. So you can update the pageLoader()
function with whichever approach you want to load the content
function pageLoader(pagename)
"use strict";
var pages =
"foo": function()
$("#main").html(`<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"bar": function()
$("#main").html(`<h1 id="title">Page-Bar</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"contact": function()
$("#main").html(`<h1 id="title">Page-Contact</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"about": function()
$("#main").html(`<h1 id="title">Page-about-Two</h1></div>
<p>Some text about about about about.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
;
//call the page
if (pages.hasOwnProperty(pagename))
pages[pagename].call(this);
//call on page load for the first time
$(document).ready(function()
pageLoader("foo");
);
//use event delegation and bind the h3
$(document).on('click', 'h3', function()
let pagename = $(this).prop("id");
pageLoader(pagename);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="main"></div>
Hope this helps
add a comment |Â
up vote
1
down vote
accepted
up vote
1
down vote
accepted
Well if you use event delegation
on the <h3>
element you won't have to care for how many pages or links there are just bind it once on page load and it will take care of the rest assuming the links are based on h3
following your given example so that solves your main problem.
$(document).on('click', 'h3', function()
//do your thing
);
Since you have stated that you have an external source to generate the HTML I would create a function pageloader()
to load the content.if that is going to be an ajax function to get the content or a template file using mutache.js
or any other.
function pageLoader()
$.ajax(
url:'path/to/content/',
//other params...
success:function(data)
//load content here
$("#map").html(data);
,
);
But if that content was to be used in the same scenario as you have provided
then you have the option of creating object literals
for keeping all the pages in one place and calling them from one place and when calling use hasOwnProperty()
to avoid inherited Properties.
var pages=
pagename:function()
//load the content of the page
,
pagename2:function()
//load content of the page
In this case What you need to consider is that you either keep the id
of the h3
similar to the page name or define a data-attribute on the h3 and add the page name there, as we would be passing that id
/data-attribute
as a parameter to our pageLoader()
function which holds all our pages and calls them too why because we will create the properties of the object literal as page name and then load them.
Below is a demonstration although I cant create an ajax example to show what I talked about in the first section, i will use the object literal section to create a demo since your actual concern was the binding of the links and not how to load the content, and that is covered. So you can update the pageLoader()
function with whichever approach you want to load the content
function pageLoader(pagename)
"use strict";
var pages =
"foo": function()
$("#main").html(`<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"bar": function()
$("#main").html(`<h1 id="title">Page-Bar</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"contact": function()
$("#main").html(`<h1 id="title">Page-Contact</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"about": function()
$("#main").html(`<h1 id="title">Page-about-Two</h1></div>
<p>Some text about about about about.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
;
//call the page
if (pages.hasOwnProperty(pagename))
pages[pagename].call(this);
//call on page load for the first time
$(document).ready(function()
pageLoader("foo");
);
//use event delegation and bind the h3
$(document).on('click', 'h3', function()
let pagename = $(this).prop("id");
pageLoader(pagename);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="main"></div>
Hope this helps
Well if you use event delegation
on the <h3>
element you won't have to care for how many pages or links there are just bind it once on page load and it will take care of the rest assuming the links are based on h3
following your given example so that solves your main problem.
$(document).on('click', 'h3', function()
//do your thing
);
Since you have stated that you have an external source to generate the HTML I would create a function pageloader()
to load the content.if that is going to be an ajax function to get the content or a template file using mutache.js
or any other.
function pageLoader()
$.ajax(
url:'path/to/content/',
//other params...
success:function(data)
//load content here
$("#map").html(data);
,
);
But if that content was to be used in the same scenario as you have provided
then you have the option of creating object literals
for keeping all the pages in one place and calling them from one place and when calling use hasOwnProperty()
to avoid inherited Properties.
var pages=
pagename:function()
//load the content of the page
,
pagename2:function()
//load content of the page
In this case What you need to consider is that you either keep the id
of the h3
similar to the page name or define a data-attribute on the h3 and add the page name there, as we would be passing that id
/data-attribute
as a parameter to our pageLoader()
function which holds all our pages and calls them too why because we will create the properties of the object literal as page name and then load them.
Below is a demonstration although I cant create an ajax example to show what I talked about in the first section, i will use the object literal section to create a demo since your actual concern was the binding of the links and not how to load the content, and that is covered. So you can update the pageLoader()
function with whichever approach you want to load the content
function pageLoader(pagename)
"use strict";
var pages =
"foo": function()
$("#main").html(`<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"bar": function()
$("#main").html(`<h1 id="title">Page-Bar</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"contact": function()
$("#main").html(`<h1 id="title">Page-Contact</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"about": function()
$("#main").html(`<h1 id="title">Page-about-Two</h1></div>
<p>Some text about about about about.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
;
//call the page
if (pages.hasOwnProperty(pagename))
pages[pagename].call(this);
//call on page load for the first time
$(document).ready(function()
pageLoader("foo");
);
//use event delegation and bind the h3
$(document).on('click', 'h3', function()
let pagename = $(this).prop("id");
pageLoader(pagename);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="main"></div>
Hope this helps
function pageLoader(pagename)
"use strict";
var pages =
"foo": function()
$("#main").html(`<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"bar": function()
$("#main").html(`<h1 id="title">Page-Bar</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"contact": function()
$("#main").html(`<h1 id="title">Page-Contact</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"about": function()
$("#main").html(`<h1 id="title">Page-about-Two</h1></div>
<p>Some text about about about about.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
;
//call the page
if (pages.hasOwnProperty(pagename))
pages[pagename].call(this);
//call on page load for the first time
$(document).ready(function()
pageLoader("foo");
);
//use event delegation and bind the h3
$(document).on('click', 'h3', function()
let pagename = $(this).prop("id");
pageLoader(pagename);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="main"></div>
function pageLoader(pagename)
"use strict";
var pages =
"foo": function()
$("#main").html(`<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"bar": function()
$("#main").html(`<h1 id="title">Page-Bar</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"contact": function()
$("#main").html(`<h1 id="title">Page-Contact</h1></div>
<p>Some text about conatct bla bla bla.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
,
"about": function()
$("#main").html(`<h1 id="title">Page-about-Two</h1></div>
<p>Some text about about about about.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`);
;
//call the page
if (pages.hasOwnProperty(pagename))
pages[pagename].call(this);
//call on page load for the first time
$(document).ready(function()
pageLoader("foo");
);
//use event delegation and bind the h3
$(document).on('click', 'h3', function()
let pagename = $(this).prop("id");
pageLoader(pagename);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div id="main"></div>
answered Feb 7 at 1:25
Muhammad Omer Aslam
171110
171110
add a comment |Â
add a comment |Â
up vote
2
down vote
The code pattern:
$("#main").html("");
$("#main").append(bar);
can be simplified to:
$("#main").html(bar);
In general, the code is, clear, easy to read, and well organized. All the decisions among alternatives are reasonable and straight forward. Doing the simplest thing that might work is a good approach because it avoids adding unneeded complexity to the necessary complexity.
The use of template literals rather than strings might be unnecessary complexity because the templating features are not used. Support for template literals is not universal among browsers. The page may break for no reason for some people. To put it another way, using template literals rather than strings creates potential broken pages for no gain in browsers that support template literals.
yes, I agree on the template literals I literally didn't notice those were backticks when copied from the OP, although they are clearlymultiline
and should have been spotted, but those backticks can be replaced withdouble
orsingle
quotes and then useMustache.js
library to render the template along with the variables/placeholder to parse. But thanks for the heads up , i appreciate.
â Muhammad Omer Aslam
Feb 7 at 17:24
add a comment |Â
up vote
2
down vote
The code pattern:
$("#main").html("");
$("#main").append(bar);
can be simplified to:
$("#main").html(bar);
In general, the code is, clear, easy to read, and well organized. All the decisions among alternatives are reasonable and straight forward. Doing the simplest thing that might work is a good approach because it avoids adding unneeded complexity to the necessary complexity.
The use of template literals rather than strings might be unnecessary complexity because the templating features are not used. Support for template literals is not universal among browsers. The page may break for no reason for some people. To put it another way, using template literals rather than strings creates potential broken pages for no gain in browsers that support template literals.
yes, I agree on the template literals I literally didn't notice those were backticks when copied from the OP, although they are clearlymultiline
and should have been spotted, but those backticks can be replaced withdouble
orsingle
quotes and then useMustache.js
library to render the template along with the variables/placeholder to parse. But thanks for the heads up , i appreciate.
â Muhammad Omer Aslam
Feb 7 at 17:24
add a comment |Â
up vote
2
down vote
up vote
2
down vote
The code pattern:
$("#main").html("");
$("#main").append(bar);
can be simplified to:
$("#main").html(bar);
In general, the code is, clear, easy to read, and well organized. All the decisions among alternatives are reasonable and straight forward. Doing the simplest thing that might work is a good approach because it avoids adding unneeded complexity to the necessary complexity.
The use of template literals rather than strings might be unnecessary complexity because the templating features are not used. Support for template literals is not universal among browsers. The page may break for no reason for some people. To put it another way, using template literals rather than strings creates potential broken pages for no gain in browsers that support template literals.
The code pattern:
$("#main").html("");
$("#main").append(bar);
can be simplified to:
$("#main").html(bar);
In general, the code is, clear, easy to read, and well organized. All the decisions among alternatives are reasonable and straight forward. Doing the simplest thing that might work is a good approach because it avoids adding unneeded complexity to the necessary complexity.
The use of template literals rather than strings might be unnecessary complexity because the templating features are not used. Support for template literals is not universal among browsers. The page may break for no reason for some people. To put it another way, using template literals rather than strings creates potential broken pages for no gain in browsers that support template literals.
edited Feb 7 at 14:14
answered Feb 7 at 14:08
ben rudgers
2,355522
2,355522
yes, I agree on the template literals I literally didn't notice those were backticks when copied from the OP, although they are clearlymultiline
and should have been spotted, but those backticks can be replaced withdouble
orsingle
quotes and then useMustache.js
library to render the template along with the variables/placeholder to parse. But thanks for the heads up , i appreciate.
â Muhammad Omer Aslam
Feb 7 at 17:24
add a comment |Â
yes, I agree on the template literals I literally didn't notice those were backticks when copied from the OP, although they are clearlymultiline
and should have been spotted, but those backticks can be replaced withdouble
orsingle
quotes and then useMustache.js
library to render the template along with the variables/placeholder to parse. But thanks for the heads up , i appreciate.
â Muhammad Omer Aslam
Feb 7 at 17:24
yes, I agree on the template literals I literally didn't notice those were backticks when copied from the OP, although they are clearly
multiline
and should have been spotted, but those backticks can be replaced with double
or single
quotes and then use Mustache.js
library to render the template along with the variables/placeholder to parse. But thanks for the heads up , i appreciate.â Muhammad Omer Aslam
Feb 7 at 17:24
yes, I agree on the template literals I literally didn't notice those were backticks when copied from the OP, although they are clearly
multiline
and should have been spotted, but those backticks can be replaced with double
or single
quotes and then use Mustache.js
library to render the template along with the variables/placeholder to parse. But thanks for the heads up , i appreciate.â Muhammad Omer Aslam
Feb 7 at 17:24
add a comment |Â
up vote
1
down vote
Ben is correct- the code is mostly clear and easy to read, though there is some redundancy (e.g. $("#main").append(foo);
, and even just $("#main")
has many occurences).
I agree with Muhammad that Event delegation is a great way to simplify the logic.
Another aspect to consider is that every time $()
appears with a CSS selector (e.g. $("#main")
) a DOM look-up is performed. This can be optimized by storing that DOM reference in a variable and then substituting that variable throughout the rest of the code. For more on this topic and other optimizations, see the section Cache DOM Lookups on this post. I know that post begins by bashing jQuery but if your goal is to use it then by all means keep it. However, you might consider alternatives - see youmightnotneedjquery.com.
So you could define a constant:
const mainElement = $("#main");
And then use that everywhere else:
mainElement.append(foo);
mainElement.on('click','h3#bar_link', function ()
mainElement.html(bar); //take advice from Ben
);//... and so on ...
Additionally, the shortcut .click()
could be used in place of .on('click', handler)
. Then to use event delegation, check the attributes of the target of the event. For example, the click handler will receive the Event object, which has the target property. The id property of that target can be fetched using .attr('id')
and that could be used to determine which element was clicked and thus which action to take. Or .is()
could be used to determine if the target matches a CSS selector.
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
const mainElement = $("#main");
mainElement.append(foo);
mainElement.click(function(clickEvent)
const clickedId = $(clickEvent.target).attr('id');
if (clickedId)
if (clickedId == 'bar_link')
mainElement.html(bar);
else
mainElement.html(foo);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<div id="main"></div>
I do like how Muhammad laid out the pages
object in pageLoader()
to map id selectors to HTML strings. However, the function calls could be removed from the mapping, and move that call to .html()
to a single call later in pageLoader()
, like the snippet below:
var pages =
"foo": `<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`,
...
;
if (pages.hasOwnProperty(pagename))
$('#main').html(pages[pagename]);
That way there is only one place .html()
is called instead of once per each property of the object.
1
i definitely missed these 2 things you mentioned above all moving the$('#main').html()
outside is way better approach than the way i used. thanks for the heads-up. it was the first ever review i submitted so am all ears to suggestions.
â Muhammad Omer Aslam
Feb 7 at 17:13
I have been on StackOverflow for quite some time but as I told you that this was my first review so wanted to ask that if I could update the answer according to the suggestions by referring you and Ben both about the points you raised and change the code or I cannot? we can do that on StackOverflow but don't know about code review if it is allowed or not ?
â Muhammad Omer Aslam
Feb 7 at 17:28
Do you mean you want to update (the code in) your answer? If so, there should be a link below it on the left side (see this)... otherwise if you mean the OPs code then know that it should not be updated, lest our answers be invalidated
â Sam Onela
Feb 7 at 17:33
1
Haha obviously not OP's code at least I am used to StackOverflow rules that much:D
, i wanted to just know if i can update the code in my own post according to the suggestions you both gave me, thanks for informing.
â Muhammad Omer Aslam
Feb 7 at 17:37
just one thing more rather than removing the function call from the object literals should'nt it be better to keep it that way, because if I replace the backtick with a single quote and use theMustach.js
to render my templates and then I would have to return the HTML after rendering, because i think @Ben has a reasonable reason to replace the backticks with single or double quotes
â Muhammad Omer Aslam
Feb 7 at 17:53
 |Â
show 2 more comments
up vote
1
down vote
Ben is correct- the code is mostly clear and easy to read, though there is some redundancy (e.g. $("#main").append(foo);
, and even just $("#main")
has many occurences).
I agree with Muhammad that Event delegation is a great way to simplify the logic.
Another aspect to consider is that every time $()
appears with a CSS selector (e.g. $("#main")
) a DOM look-up is performed. This can be optimized by storing that DOM reference in a variable and then substituting that variable throughout the rest of the code. For more on this topic and other optimizations, see the section Cache DOM Lookups on this post. I know that post begins by bashing jQuery but if your goal is to use it then by all means keep it. However, you might consider alternatives - see youmightnotneedjquery.com.
So you could define a constant:
const mainElement = $("#main");
And then use that everywhere else:
mainElement.append(foo);
mainElement.on('click','h3#bar_link', function ()
mainElement.html(bar); //take advice from Ben
);//... and so on ...
Additionally, the shortcut .click()
could be used in place of .on('click', handler)
. Then to use event delegation, check the attributes of the target of the event. For example, the click handler will receive the Event object, which has the target property. The id property of that target can be fetched using .attr('id')
and that could be used to determine which element was clicked and thus which action to take. Or .is()
could be used to determine if the target matches a CSS selector.
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
const mainElement = $("#main");
mainElement.append(foo);
mainElement.click(function(clickEvent)
const clickedId = $(clickEvent.target).attr('id');
if (clickedId)
if (clickedId == 'bar_link')
mainElement.html(bar);
else
mainElement.html(foo);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<div id="main"></div>
I do like how Muhammad laid out the pages
object in pageLoader()
to map id selectors to HTML strings. However, the function calls could be removed from the mapping, and move that call to .html()
to a single call later in pageLoader()
, like the snippet below:
var pages =
"foo": `<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`,
...
;
if (pages.hasOwnProperty(pagename))
$('#main').html(pages[pagename]);
That way there is only one place .html()
is called instead of once per each property of the object.
1
i definitely missed these 2 things you mentioned above all moving the$('#main').html()
outside is way better approach than the way i used. thanks for the heads-up. it was the first ever review i submitted so am all ears to suggestions.
â Muhammad Omer Aslam
Feb 7 at 17:13
I have been on StackOverflow for quite some time but as I told you that this was my first review so wanted to ask that if I could update the answer according to the suggestions by referring you and Ben both about the points you raised and change the code or I cannot? we can do that on StackOverflow but don't know about code review if it is allowed or not ?
â Muhammad Omer Aslam
Feb 7 at 17:28
Do you mean you want to update (the code in) your answer? If so, there should be a link below it on the left side (see this)... otherwise if you mean the OPs code then know that it should not be updated, lest our answers be invalidated
â Sam Onela
Feb 7 at 17:33
1
Haha obviously not OP's code at least I am used to StackOverflow rules that much:D
, i wanted to just know if i can update the code in my own post according to the suggestions you both gave me, thanks for informing.
â Muhammad Omer Aslam
Feb 7 at 17:37
just one thing more rather than removing the function call from the object literals should'nt it be better to keep it that way, because if I replace the backtick with a single quote and use theMustach.js
to render my templates and then I would have to return the HTML after rendering, because i think @Ben has a reasonable reason to replace the backticks with single or double quotes
â Muhammad Omer Aslam
Feb 7 at 17:53
 |Â
show 2 more comments
up vote
1
down vote
up vote
1
down vote
Ben is correct- the code is mostly clear and easy to read, though there is some redundancy (e.g. $("#main").append(foo);
, and even just $("#main")
has many occurences).
I agree with Muhammad that Event delegation is a great way to simplify the logic.
Another aspect to consider is that every time $()
appears with a CSS selector (e.g. $("#main")
) a DOM look-up is performed. This can be optimized by storing that DOM reference in a variable and then substituting that variable throughout the rest of the code. For more on this topic and other optimizations, see the section Cache DOM Lookups on this post. I know that post begins by bashing jQuery but if your goal is to use it then by all means keep it. However, you might consider alternatives - see youmightnotneedjquery.com.
So you could define a constant:
const mainElement = $("#main");
And then use that everywhere else:
mainElement.append(foo);
mainElement.on('click','h3#bar_link', function ()
mainElement.html(bar); //take advice from Ben
);//... and so on ...
Additionally, the shortcut .click()
could be used in place of .on('click', handler)
. Then to use event delegation, check the attributes of the target of the event. For example, the click handler will receive the Event object, which has the target property. The id property of that target can be fetched using .attr('id')
and that could be used to determine which element was clicked and thus which action to take. Or .is()
could be used to determine if the target matches a CSS selector.
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
const mainElement = $("#main");
mainElement.append(foo);
mainElement.click(function(clickEvent)
const clickedId = $(clickEvent.target).attr('id');
if (clickedId)
if (clickedId == 'bar_link')
mainElement.html(bar);
else
mainElement.html(foo);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<div id="main"></div>
I do like how Muhammad laid out the pages
object in pageLoader()
to map id selectors to HTML strings. However, the function calls could be removed from the mapping, and move that call to .html()
to a single call later in pageLoader()
, like the snippet below:
var pages =
"foo": `<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`,
...
;
if (pages.hasOwnProperty(pagename))
$('#main').html(pages[pagename]);
That way there is only one place .html()
is called instead of once per each property of the object.
Ben is correct- the code is mostly clear and easy to read, though there is some redundancy (e.g. $("#main").append(foo);
, and even just $("#main")
has many occurences).
I agree with Muhammad that Event delegation is a great way to simplify the logic.
Another aspect to consider is that every time $()
appears with a CSS selector (e.g. $("#main")
) a DOM look-up is performed. This can be optimized by storing that DOM reference in a variable and then substituting that variable throughout the rest of the code. For more on this topic and other optimizations, see the section Cache DOM Lookups on this post. I know that post begins by bashing jQuery but if your goal is to use it then by all means keep it. However, you might consider alternatives - see youmightnotneedjquery.com.
So you could define a constant:
const mainElement = $("#main");
And then use that everywhere else:
mainElement.append(foo);
mainElement.on('click','h3#bar_link', function ()
mainElement.html(bar); //take advice from Ben
);//... and so on ...
Additionally, the shortcut .click()
could be used in place of .on('click', handler)
. Then to use event delegation, check the attributes of the target of the event. For example, the click handler will receive the Event object, which has the target property. The id property of that target can be fetched using .attr('id')
and that could be used to determine which element was clicked and thus which action to take. Or .is()
could be used to determine if the target matches a CSS selector.
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
const mainElement = $("#main");
mainElement.append(foo);
mainElement.click(function(clickEvent)
const clickedId = $(clickEvent.target).attr('id');
if (clickedId)
if (clickedId == 'bar_link')
mainElement.html(bar);
else
mainElement.html(foo);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<div id="main"></div>
I do like how Muhammad laid out the pages
object in pageLoader()
to map id selectors to HTML strings. However, the function calls could be removed from the mapping, and move that call to .html()
to a single call later in pageLoader()
, like the snippet below:
var pages =
"foo": `<h1 id="title">Page-Foo</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="foo">Link to Foo</h3><h3 id="bar">Link to bar</h3><h3 id="contact">Link to Contact</h3><h3 id="about">Link to About</h3>`,
...
;
if (pages.hasOwnProperty(pagename))
$('#main').html(pages[pagename]);
That way there is only one place .html()
is called instead of once per each property of the object.
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
const mainElement = $("#main");
mainElement.append(foo);
mainElement.click(function(clickEvent)
const clickedId = $(clickEvent.target).attr('id');
if (clickedId)
if (clickedId == 'bar_link')
mainElement.html(bar);
else
mainElement.html(foo);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<div id="main"></div>
const foo = `<h1 id="title">Page-Foo-One</h1></div>
<p>Some text about foo foo foo.</p>
<h3 id="bar_link">Link to bar</h3>`;
const bar = `<h1 id="title">Page-Bar-Two</h1></div>
<p>Some text about bar bar bar.</p>
<h3 id="foo_link">Link to foo</h3>`;
const mainElement = $("#main");
mainElement.append(foo);
mainElement.click(function(clickEvent)
const clickedId = $(clickEvent.target).attr('id');
if (clickedId)
if (clickedId == 'bar_link')
mainElement.html(bar);
else
mainElement.html(foo);
);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<div id="main"></div>
edited Feb 7 at 17:39
answered Feb 7 at 16:49
Sam Onela
5,88461545
5,88461545
1
i definitely missed these 2 things you mentioned above all moving the$('#main').html()
outside is way better approach than the way i used. thanks for the heads-up. it was the first ever review i submitted so am all ears to suggestions.
â Muhammad Omer Aslam
Feb 7 at 17:13
I have been on StackOverflow for quite some time but as I told you that this was my first review so wanted to ask that if I could update the answer according to the suggestions by referring you and Ben both about the points you raised and change the code or I cannot? we can do that on StackOverflow but don't know about code review if it is allowed or not ?
â Muhammad Omer Aslam
Feb 7 at 17:28
Do you mean you want to update (the code in) your answer? If so, there should be a link below it on the left side (see this)... otherwise if you mean the OPs code then know that it should not be updated, lest our answers be invalidated
â Sam Onela
Feb 7 at 17:33
1
Haha obviously not OP's code at least I am used to StackOverflow rules that much:D
, i wanted to just know if i can update the code in my own post according to the suggestions you both gave me, thanks for informing.
â Muhammad Omer Aslam
Feb 7 at 17:37
just one thing more rather than removing the function call from the object literals should'nt it be better to keep it that way, because if I replace the backtick with a single quote and use theMustach.js
to render my templates and then I would have to return the HTML after rendering, because i think @Ben has a reasonable reason to replace the backticks with single or double quotes
â Muhammad Omer Aslam
Feb 7 at 17:53
 |Â
show 2 more comments
1
i definitely missed these 2 things you mentioned above all moving the$('#main').html()
outside is way better approach than the way i used. thanks for the heads-up. it was the first ever review i submitted so am all ears to suggestions.
â Muhammad Omer Aslam
Feb 7 at 17:13
I have been on StackOverflow for quite some time but as I told you that this was my first review so wanted to ask that if I could update the answer according to the suggestions by referring you and Ben both about the points you raised and change the code or I cannot? we can do that on StackOverflow but don't know about code review if it is allowed or not ?
â Muhammad Omer Aslam
Feb 7 at 17:28
Do you mean you want to update (the code in) your answer? If so, there should be a link below it on the left side (see this)... otherwise if you mean the OPs code then know that it should not be updated, lest our answers be invalidated
â Sam Onela
Feb 7 at 17:33
1
Haha obviously not OP's code at least I am used to StackOverflow rules that much:D
, i wanted to just know if i can update the code in my own post according to the suggestions you both gave me, thanks for informing.
â Muhammad Omer Aslam
Feb 7 at 17:37
just one thing more rather than removing the function call from the object literals should'nt it be better to keep it that way, because if I replace the backtick with a single quote and use theMustach.js
to render my templates and then I would have to return the HTML after rendering, because i think @Ben has a reasonable reason to replace the backticks with single or double quotes
â Muhammad Omer Aslam
Feb 7 at 17:53
1
1
i definitely missed these 2 things you mentioned above all moving the
$('#main').html()
outside is way better approach than the way i used. thanks for the heads-up. it was the first ever review i submitted so am all ears to suggestions.â Muhammad Omer Aslam
Feb 7 at 17:13
i definitely missed these 2 things you mentioned above all moving the
$('#main').html()
outside is way better approach than the way i used. thanks for the heads-up. it was the first ever review i submitted so am all ears to suggestions.â Muhammad Omer Aslam
Feb 7 at 17:13
I have been on StackOverflow for quite some time but as I told you that this was my first review so wanted to ask that if I could update the answer according to the suggestions by referring you and Ben both about the points you raised and change the code or I cannot? we can do that on StackOverflow but don't know about code review if it is allowed or not ?
â Muhammad Omer Aslam
Feb 7 at 17:28
I have been on StackOverflow for quite some time but as I told you that this was my first review so wanted to ask that if I could update the answer according to the suggestions by referring you and Ben both about the points you raised and change the code or I cannot? we can do that on StackOverflow but don't know about code review if it is allowed or not ?
â Muhammad Omer Aslam
Feb 7 at 17:28
Do you mean you want to update (the code in) your answer? If so, there should be a link below it on the left side (see this)... otherwise if you mean the OPs code then know that it should not be updated, lest our answers be invalidated
â Sam Onela
Feb 7 at 17:33
Do you mean you want to update (the code in) your answer? If so, there should be a link below it on the left side (see this)... otherwise if you mean the OPs code then know that it should not be updated, lest our answers be invalidated
â Sam Onela
Feb 7 at 17:33
1
1
Haha obviously not OP's code at least I am used to StackOverflow rules that much
:D
, i wanted to just know if i can update the code in my own post according to the suggestions you both gave me, thanks for informing.â Muhammad Omer Aslam
Feb 7 at 17:37
Haha obviously not OP's code at least I am used to StackOverflow rules that much
:D
, i wanted to just know if i can update the code in my own post according to the suggestions you both gave me, thanks for informing.â Muhammad Omer Aslam
Feb 7 at 17:37
just one thing more rather than removing the function call from the object literals should'nt it be better to keep it that way, because if I replace the backtick with a single quote and use the
Mustach.js
to render my templates and then I would have to return the HTML after rendering, because i think @Ben has a reasonable reason to replace the backticks with single or double quotesâ Muhammad Omer Aslam
Feb 7 at 17:53
just one thing more rather than removing the function call from the object literals should'nt it be better to keep it that way, because if I replace the backtick with a single quote and use the
Mustach.js
to render my templates and then I would have to return the HTML after rendering, because i think @Ben has a reasonable reason to replace the backticks with single or double quotesâ Muhammad Omer Aslam
Feb 7 at 17:53
 |Â
show 2 more comments
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186921%2fjquery-onclick-to-toggle-webpage-content%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password