Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Camera flash addition #25

Merged
merged 9 commits into from
Jun 22, 2024
Merged

Camera flash addition #25

merged 9 commits into from
Jun 22, 2024

Conversation

suryabhaiin
Copy link
Contributor

Added new function, Flash light with on/off option and Fixed Photo URL copy to clipboard after photo capture

Camera flash added with on/off option
Clipboard Copy URL fix
Comment on lines 46 to 61
function copyToClipboard(text) {
text = text.replace(/"/g, '');
text = text.trim();
var textarea = document.createElement('textarea');
textarea.value = text;
textarea.style.position = 'absolute';
textarea.style.left = '-9999px';
document.body.appendChild(textarea);
textarea.select();
try {
document.execCommand('copy');
} catch (err) {
console.error('Unable to copy to clipboard:', err);
}
document.body.removeChild(textarea);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make a "copy to clipboard" function? There is a clipboard API built into the browser you have replaced?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added this function because clipboard API not work & throws following error
Uncaught (in promise) NotAllowedError: Document is not focused. (@ps-camera/client/nui/index.html:0)

Comment on lines 194 to 198
if doFlash then
SendNUIMessage({action = "toggleFlash", status = '<b style="color:green;">ON</b>'})
else
SendNUIMessage({action = "toggleFlash", status = '<b style="color:red;">OFF</b>'})
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor this code. It has cyber security concerns since people can intercept the status which gets inserted raw into the DOM, making it possible for hacking to inject whatever code they want into the user's UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, i will rework on this code.

@@ -57,7 +79,10 @@ $(document).ready(function () {
} else if (event.data.action === 'openPhoto') {
open(event.data.image, event.data.location);
} else if (event.data.action === 'SavePic') {
navigator.clipboard.writeText(str);
copyToClipboard(event.data.pic);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert line to use Clipboard API.

Comment on lines 63 to 66
function toggleflash(status)
{
$('#flashstatus').html(status);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor this code to alter some already defined markup in the HTML instead of injecting HTML directly. This is very bad practice if you don't know what the user will be sending to you.

README.md Outdated

# Preview
* Camera Overlay
![image](https://user-images.githubusercontent.com/82112471/231553020-f5061241-e04a-462e-8266-a48b8efc9884.png)

* Picture Overlay
![image](https://user-images.githubusercontent.com/82112471/231553182-fd15c5f7-b908-42f7-a8d6-93185fd6e3c2.png)

* Picture With And Without Flash<br>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not use HTML tags in a markdown file.

@@ -139,6 +139,25 @@ function SetLocation()
end
end

local doFlash = false
function FlashLightEffect()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function can be made local.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sure.

@xFutte xFutte changed the title Improvment Camera flash addition Mar 17, 2024
Some requested changed and bug fixes
Reverted to use Clipboard API but str is undefined so replaced with
Copy existing Photos URL button added
@suryabhaiin suryabhaiin requested a review from xFutte March 19, 2024 05:54
@@ -1,5 +1,5 @@
var displayPicture = false;

var tempsrc = 'dada';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update variable name and use let instead. Also, remove the value "dada"

Initial value and veriable data type change
@suryabhaiin
Copy link
Contributor Author

Requested changes done

@MonkeyWhisper MonkeyWhisper requested a review from xFutte June 5, 2024 20:16
@@ -57,7 +73,9 @@ $(document).ready(function () {
} else if (event.data.action === 'openPhoto') {
open(event.data.image, event.data.location);
} else if (event.data.action === 'SavePic') {
navigator.clipboard.writeText(str);
navigator.clipboard.writeText(event.data.pic);
Copy link
Member

@xFutte xFutte Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event.data.pic is missing to be sanitized for security reasons. Create a method to sanitize it to prevent in-game exploits and cyber attacks on the players PC.


$(document).ready(function() {
$('#copynow').on('click', function() {
navigator.clipboard.writeText(tempsrc);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tempsrc needs to be sanitized.

@@ -76,7 +78,7 @@
<div class="status-bar">
<div class="battery-life">
<i class="fa-solid fa-battery-three-quarters icon"></i>
<span>69%</span>
<span>69%</span>&nbsp;&nbsp;<b id="flashstatus" class="off"><i class="fa-solid fa-bolt"></i></b>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding the &nbsp; twice, just add a margin-left to the flashstatus selector in the CSS.

sanitize done
UI Update
UI Update
@suryabhaiin
Copy link
Contributor Author

Requested changes done

@xFutte xFutte merged commit f81b228 into Project-Sloth:main Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants