Calculating absolute position in %
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
1
down vote
favorite
I'm making a website for a friend (just for fun) where we have to move a lot of svg images in absolute positions. To make it easier I wrote some code to calculate the % position of each element so we can do it faster.
I'm wondering what you think about it and what can I do to improve it. And also any improvements to my style of coding are welcome.
<html>
<head>
<!-- Just some style to differentiate the object -->
<style>
.draggable
width: 10px;
height: 10px;
background: red;
</style>
</head>
<body>
<!-- The item that we will be moving around -->
<div class="draggable"></div>
<script>
class Mover
constructor(htmlObject,parent, position)
// Store the object to move
this.htmlObject = htmlObject;
// The parent of the object
this.parent = parent;
// The type of position : 'relative'/'absolute'
this.position = position;
// Modify the elements and start the listeners
this.init();
init()
// Make the item draggable
this.htmlObject.draggable = 'true';
// Set its position type to the user input
this.htmlObject.style.position = this.position;
// Modify its zIndex to make sure that you can always see where it is.
this.htmlObject.style.zIndex = '9999';
// Add some border just make it easier to find the item.
this.htmlObject.style.border = '1px solid black';
// Add the listeners for the drop and drag events to the parent
this.parent.ondrop = (event)=> this.drop(event) ;
this.parent.ondragover = (event)=> this.allowDrop(event) ;
drop(e)
e.preventDefault();
let pos = this.calculatePos(e.x, e.y);
// Set the object position equal to the event x and y pos in %
this.htmlObject.style.top = pos.h+'%';
this.htmlObject.style.left = pos.w+'%';
allowDrop(e)
// Allow to drop.
e.preventDefault();
calculatePos(w,h)
// Get the parent Width and Height
let mx = this.parent.offsetWidth;
let mh = this.parent.offsetHeight;
// -- Just testing stuff --
// let mx = innerWidth;
// let mh = innerHeight;
// ------------------------
// Calculate the % based on the parent size.
return w : (w/mx) * 100, h : (h/mh) * 100
// Initialize the movable item.
const draggableElement = new Mover(document.querySelector('.dragable'),document.querySelector('body'),'absolute')
</script>
</body>
javascript html css
edited May 10 at 4:50
Igor Soloydenko
2,697827
asked May 9 at 19:58
ithan
253
add a comment |Â
up vote
1
down vote
favorite
I'm making a website for a friend (just for fun) where we have to move a lot of svg images in absolute positions. To make it easier I wrote some code to calculate the % position of each element so we can do it faster.
I'm wondering what you think about it and what can I do to improve it. And also any improvements to my style of coding are welcome.
<html>
<head>
<!-- Just some style to differentiate the object -->
<style>
.draggable
width: 10px;
height: 10px;
background: red;
</style>
</head>
<body>
<!-- The item that we will be moving around -->
<div class="draggable"></div>
<script>
class Mover
constructor(htmlObject,parent, position)
// Store the object to move
this.htmlObject = htmlObject;
// The parent of the object
this.parent = parent;
// The type of position : 'relative'/'absolute'
this.position = position;
// Modify the elements and start the listeners
this.init();
init()
// Make the item draggable
this.htmlObject.draggable = 'true';
// Set its position type to the user input
this.htmlObject.style.position = this.position;
// Modify its zIndex to make sure that you can always see where it is.
this.htmlObject.style.zIndex = '9999';
// Add some border just make it easier to find the item.
this.htmlObject.style.border = '1px solid black';
// Add the listeners for the drop and drag events to the parent
this.parent.ondrop = (event)=> this.drop(event) ;
this.parent.ondragover = (event)=> this.allowDrop(event) ;
drop(e)
e.preventDefault();
let pos = this.calculatePos(e.x, e.y);
// Set the object position equal to the event x and y pos in %
this.htmlObject.style.top = pos.h+'%';
this.htmlObject.style.left = pos.w+'%';
allowDrop(e)
// Allow to drop.
e.preventDefault();
calculatePos(w,h)
// Get the parent Width and Height
let mx = this.parent.offsetWidth;
let mh = this.parent.offsetHeight;
// -- Just testing stuff --
// let mx = innerWidth;
// let mh = innerHeight;
// ------------------------
// Calculate the % based on the parent size.
return w : (w/mx) * 100, h : (h/mh) * 100
// Initialize the movable item.
const draggableElement = new Mover(document.querySelector('.dragable'),document.querySelector('body'),'absolute')
</script>
</body>
javascript html css
edited May 10 at 4:50
Igor Soloydenko
2,697827
asked May 9 at 19:58
ithan
253
Is this code complete? I seedraggableElementis assigned a newly created instance ofMover; however I do not see it invoked/referred/used anywhere.
– Igor Soloydenko
May 10 at 4:51
In the current code you only need to declare it, the constructor calls the init function, probably i should use draggableElement.init() instead of having the init at the constructor!
– ithan
May 10 at 11:47
add a comment |Â
up vote
1
down vote
favorite
up vote
1
down vote
favorite
I'm making a website for a friend (just for fun) where we have to move a lot of svg images in absolute positions. To make it easier I wrote some code to calculate the % position of each element so we can do it faster.
I'm wondering what you think about it and what can I do to improve it. And also any improvements to my style of coding are welcome.
<html>
<head>
<!-- Just some style to differentiate the object -->
<style>
.draggable
width: 10px;
height: 10px;
background: red;
</style>
</head>
<body>
<!-- The item that we will be moving around -->
<div class="draggable"></div>
<script>
class Mover
constructor(htmlObject,parent, position)
// Store the object to move
this.htmlObject = htmlObject;
// The parent of the object
this.parent = parent;
// The type of position : 'relative'/'absolute'
this.position = position;
// Modify the elements and start the listeners
this.init();
init()
// Make the item draggable
this.htmlObject.draggable = 'true';
// Set its position type to the user input
this.htmlObject.style.position = this.position;
// Modify its zIndex to make sure that you can always see where it is.
this.htmlObject.style.zIndex = '9999';
// Add some border just make it easier to find the item.
this.htmlObject.style.border = '1px solid black';
// Add the listeners for the drop and drag events to the parent
this.parent.ondrop = (event)=> this.drop(event) ;
this.parent.ondragover = (event)=> this.allowDrop(event) ;
drop(e)
e.preventDefault();
let pos = this.calculatePos(e.x, e.y);
// Set the object position equal to the event x and y pos in %
this.htmlObject.style.top = pos.h+'%';
this.htmlObject.style.left = pos.w+'%';
allowDrop(e)
// Allow to drop.
e.preventDefault();
calculatePos(w,h)
// Get the parent Width and Height
let mx = this.parent.offsetWidth;
let mh = this.parent.offsetHeight;
// -- Just testing stuff --
// let mx = innerWidth;
// let mh = innerHeight;
// ------------------------
// Calculate the % based on the parent size.
return w : (w/mx) * 100, h : (h/mh) * 100
// Initialize the movable item.
const draggableElement = new Mover(document.querySelector('.dragable'),document.querySelector('body'),'absolute')
</script>
</body>
javascript html css
edited May 10 at 4:50
Igor Soloydenko
2,697827
asked May 9 at 19:58
ithan
253
I'm making a website for a friend (just for fun) where we have to move a lot of svg images in absolute positions. To make it easier I wrote some code to calculate the % position of each element so we can do it faster.
I'm wondering what you think about it and what can I do to improve it. And also any improvements to my style of coding are welcome.
<html>
<head>
<!-- Just some style to differentiate the object -->
<style>
.draggable
width: 10px;
height: 10px;
background: red;
</style>
</head>
<body>
<!-- The item that we will be moving around -->
<div class="draggable"></div>
<script>
class Mover
constructor(htmlObject,parent, position)
// Store the object to move
this.htmlObject = htmlObject;
// The parent of the object
this.parent = parent;
// The type of position : 'relative'/'absolute'
this.position = position;
// Modify the elements and start the listeners
this.init();
init()
// Make the item draggable
this.htmlObject.draggable = 'true';
// Set its position type to the user input
this.htmlObject.style.position = this.position;
// Modify its zIndex to make sure that you can always see where it is.
this.htmlObject.style.zIndex = '9999';
// Add some border just make it easier to find the item.
this.htmlObject.style.border = '1px solid black';
// Add the listeners for the drop and drag events to the parent
this.parent.ondrop = (event)=> this.drop(event) ;
this.parent.ondragover = (event)=> this.allowDrop(event) ;
drop(e)
e.preventDefault();
let pos = this.calculatePos(e.x, e.y);
// Set the object position equal to the event x and y pos in %
this.htmlObject.style.top = pos.h+'%';
this.htmlObject.style.left = pos.w+'%';
allowDrop(e)
// Allow to drop.
e.preventDefault();
calculatePos(w,h)
// Get the parent Width and Height
let mx = this.parent.offsetWidth;
let mh = this.parent.offsetHeight;
// -- Just testing stuff --
// let mx = innerWidth;
// let mh = innerHeight;
// ------------------------
// Calculate the % based on the parent size.
return w : (w/mx) * 100, h : (h/mh) * 100
// Initialize the movable item.
const draggableElement = new Mover(document.querySelector('.dragable'),document.querySelector('body'),'absolute')
</script>
</body>
javascript html css
edited May 10 at 4:50
Igor Soloydenko
2,697827
asked May 9 at 19:58
ithan
253
edited May 10 at 4:50
Igor Soloydenko
2,697827
edited May 10 at 4:50
Igor Soloydenko
2,697827
edited May 10 at 4:50
Igor Soloydenko
2,697827
2,697827
asked May 9 at 19:58
ithan
253
asked May 9 at 19:58
ithan
253
asked May 9 at 19:58
ithan
253
253
Is this code complete? I seedraggableElementis assigned a newly created instance ofMover; however I do not see it invoked/referred/used anywhere.
– Igor Soloydenko
May 10 at 4:51
In the current code you only need to declare it, the constructor calls the init function, probably i should use draggableElement.init() instead of having the init at the constructor!
– ithan
May 10 at 11:47
add a comment |Â
Is this code complete? I seedraggableElementis assigned a newly created instance ofMover; however I do not see it invoked/referred/used anywhere.
– Igor Soloydenko
May 10 at 4:51
In the current code you only need to declare it, the constructor calls the init function, probably i should use draggableElement.init() instead of having the init at the constructor!
– ithan
May 10 at 11:47
Is this code complete? I see
draggableElement is assigned a newly created instance of Mover; however I do not see it invoked/referred/used anywhere.– Igor Soloydenko
May 10 at 4:51
Is this code complete? I see
draggableElement is assigned a newly created instance of Mover; however I do not see it invoked/referred/used anywhere.– Igor Soloydenko
May 10 at 4:51
In the current code you only need to declare it, the constructor calls the init function, probably i should use draggableElement.init() instead of having the init at the constructor!
– ithan
May 10 at 11:47
In the current code you only need to declare it, the constructor calls the init function, probably i should use draggableElement.init() instead of having the init at the constructor!
– ithan
May 10 at 11:47
add a comment |Â
1 Answer
1
active
oldest
votes
up vote
1
down vote
accepted
About Constructor
It is considered a bad practice for a constructor to have side effects for multiple reasons. You can browse numerous resources online on this subject. Here are a few whys:
- The reader/maintainer of the code does not expect a constructor to have any side effects;
- Such constructor is hard or impossible to test;
- It violates a whole bunch of principles, including Principle of least astonishment, SRP.
So, yeah instead of
constructor(htmlObject,parent, position)
// ...
this.init();
you should rather create an instance and invoke the init() on it; like this
const draggableElement = new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
);
draggableElement.init();
If you are not planning to add the function that disables "moving" of an element, you may even not need a local variable holding the reference to a Mover:
new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
)
.init();
Naming Things
Naming is important. Method init() is not informative. You may want to rename it to something like enableElementDragging(). A symmetric counter part is now too easy too name – disableElementDragging().
Nano variable names are evil. What is e? An error? An exception? An event? To figure that out I need to look at a place where the function is being invoked. That's mental burden.
Same applies to pos, w, and h... If you are in VS Code or any other smart editor, it's a matter of hitting F2 and renaming it.
Names mx and mh are particularly unfortunate since they are meant to be symmetric to/consistent with w and h. Thus, mx should be mw! Also, I could not decipher what the m prefix means in both of these variables.
Just spell things out: position, width, height, parentWidth, parentHeight, calculatePosition().
answered May 10 at 14:26
Igor Soloydenko
2,697827
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
accepted
About Constructor
It is considered a bad practice for a constructor to have side effects for multiple reasons. You can browse numerous resources online on this subject. Here are a few whys:
- The reader/maintainer of the code does not expect a constructor to have any side effects;
- Such constructor is hard or impossible to test;
- It violates a whole bunch of principles, including Principle of least astonishment, SRP.
So, yeah instead of
constructor(htmlObject,parent, position)
// ...
this.init();
you should rather create an instance and invoke the init() on it; like this
const draggableElement = new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
);
draggableElement.init();
If you are not planning to add the function that disables "moving" of an element, you may even not need a local variable holding the reference to a Mover:
new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
)
.init();
Naming Things
Naming is important. Method init() is not informative. You may want to rename it to something like enableElementDragging(). A symmetric counter part is now too easy too name – disableElementDragging().
Nano variable names are evil. What is e? An error? An exception? An event? To figure that out I need to look at a place where the function is being invoked. That's mental burden.
Same applies to pos, w, and h... If you are in VS Code or any other smart editor, it's a matter of hitting F2 and renaming it.
Names mx and mh are particularly unfortunate since they are meant to be symmetric to/consistent with w and h. Thus, mx should be mw! Also, I could not decipher what the m prefix means in both of these variables.
Just spell things out: position, width, height, parentWidth, parentHeight, calculatePosition().
answered May 10 at 14:26
Igor Soloydenko
2,697827
add a comment |Â
up vote
1
down vote
accepted
About Constructor
It is considered a bad practice for a constructor to have side effects for multiple reasons. You can browse numerous resources online on this subject. Here are a few whys:
- The reader/maintainer of the code does not expect a constructor to have any side effects;
- Such constructor is hard or impossible to test;
- It violates a whole bunch of principles, including Principle of least astonishment, SRP.
So, yeah instead of
constructor(htmlObject,parent, position)
// ...
this.init();
you should rather create an instance and invoke the init() on it; like this
const draggableElement = new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
);
draggableElement.init();
If you are not planning to add the function that disables "moving" of an element, you may even not need a local variable holding the reference to a Mover:
new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
)
.init();
Naming Things
Naming is important. Method init() is not informative. You may want to rename it to something like enableElementDragging(). A symmetric counter part is now too easy too name – disableElementDragging().
Nano variable names are evil. What is e? An error? An exception? An event? To figure that out I need to look at a place where the function is being invoked. That's mental burden.
Same applies to pos, w, and h... If you are in VS Code or any other smart editor, it's a matter of hitting F2 and renaming it.
Names mx and mh are particularly unfortunate since they are meant to be symmetric to/consistent with w and h. Thus, mx should be mw! Also, I could not decipher what the m prefix means in both of these variables.
Just spell things out: position, width, height, parentWidth, parentHeight, calculatePosition().
answered May 10 at 14:26
Igor Soloydenko
2,697827
add a comment |Â
up vote
1
down vote
accepted
up vote
1
down vote
accepted
About Constructor
It is considered a bad practice for a constructor to have side effects for multiple reasons. You can browse numerous resources online on this subject. Here are a few whys:
- The reader/maintainer of the code does not expect a constructor to have any side effects;
- Such constructor is hard or impossible to test;
- It violates a whole bunch of principles, including Principle of least astonishment, SRP.
So, yeah instead of
constructor(htmlObject,parent, position)
// ...
this.init();
you should rather create an instance and invoke the init() on it; like this
const draggableElement = new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
);
draggableElement.init();
If you are not planning to add the function that disables "moving" of an element, you may even not need a local variable holding the reference to a Mover:
new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
)
.init();
Naming Things
Naming is important. Method init() is not informative. You may want to rename it to something like enableElementDragging(). A symmetric counter part is now too easy too name – disableElementDragging().
Nano variable names are evil. What is e? An error? An exception? An event? To figure that out I need to look at a place where the function is being invoked. That's mental burden.
Same applies to pos, w, and h... If you are in VS Code or any other smart editor, it's a matter of hitting F2 and renaming it.
Names mx and mh are particularly unfortunate since they are meant to be symmetric to/consistent with w and h. Thus, mx should be mw! Also, I could not decipher what the m prefix means in both of these variables.
Just spell things out: position, width, height, parentWidth, parentHeight, calculatePosition().
answered May 10 at 14:26
Igor Soloydenko
2,697827
About Constructor
It is considered a bad practice for a constructor to have side effects for multiple reasons. You can browse numerous resources online on this subject. Here are a few whys:
- The reader/maintainer of the code does not expect a constructor to have any side effects;
- Such constructor is hard or impossible to test;
- It violates a whole bunch of principles, including Principle of least astonishment, SRP.
So, yeah instead of
constructor(htmlObject,parent, position)
// ...
this.init();
you should rather create an instance and invoke the init() on it; like this
const draggableElement = new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
);
draggableElement.init();
If you are not planning to add the function that disables "moving" of an element, you may even not need a local variable holding the reference to a Mover:
new Mover(
document.querySelector('.dragable'),
document.querySelector('body'),
'absolute',
)
.init();
Naming Things
Naming is important. Method init() is not informative. You may want to rename it to something like enableElementDragging(). A symmetric counter part is now too easy too name – disableElementDragging().
Nano variable names are evil. What is e? An error? An exception? An event? To figure that out I need to look at a place where the function is being invoked. That's mental burden.
Same applies to pos, w, and h... If you are in VS Code or any other smart editor, it's a matter of hitting F2 and renaming it.
Names mx and mh are particularly unfortunate since they are meant to be symmetric to/consistent with w and h. Thus, mx should be mw! Also, I could not decipher what the m prefix means in both of these variables.
Just spell things out: position, width, height, parentWidth, parentHeight, calculatePosition().
answered May 10 at 14:26
Igor Soloydenko
2,697827
answered May 10 at 14:26
Igor Soloydenko
2,697827
answered May 10 at 14:26
Igor Soloydenko
2,697827
answered May 10 at 14:26
Igor Soloydenko
2,697827
2,697827
add a comment |Â
add a comment |Â
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%2f194045%2fcalculating-absolute-position-in%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
Is this code complete? I see
draggableElementis assigned a newly created instance ofMover; however I do not see it invoked/referred/used anywhere.– Igor Soloydenko
May 10 at 4:51
In the current code you only need to declare it, the constructor calls the init function, probably i should use draggableElement.init() instead of having the init at the constructor!
– ithan
May 10 at 11:47