diff --git a/resources/js/app.js b/resources/js/app.js index 54d724a..2877d09 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -2,6 +2,7 @@ import './bootstrap'; import './components/Main.jsx'; import Alpine from 'alpinejs'; +// todo: Put Alpine to global scope (Window scope is not good practice) window.Alpine = Alpine; Alpine.start(); diff --git a/resources/js/bootstrap.js b/resources/js/bootstrap.js index ded34c6..b70ab33 100644 --- a/resources/js/bootstrap.js +++ b/resources/js/bootstrap.js @@ -1,4 +1,9 @@ import axios from 'axios'; +// todo: Put axios to global scope (Window scope is not good practice) +/* + Instead of putting it in Window scope you can create configuration file with all settings for axios + in that file and reuse exported Axios instance where do you need it +*/ window.axios = axios; window.axios.defaults.headers.common['X-Requested-With'] = 'XMLHttpRequest'; diff --git a/resources/js/components/BroadcastEvent.jsx b/resources/js/components/BroadcastEvent.jsx index 9383a79..f307d99 100644 --- a/resources/js/components/BroadcastEvent.jsx +++ b/resources/js/components/BroadcastEvent.jsx @@ -1,3 +1,5 @@ +// todo: capital letter is not necessary here, because this is not a class or component, it's a function +// use broadcastEvent const BroadcastEvent = function ({channel, event, data}) { window.Echo.join(channel) .whisper(event, data) diff --git a/resources/js/components/ChatBox.jsx b/resources/js/components/ChatBox.jsx index 317e43c..7132fe4 100644 --- a/resources/js/components/ChatBox.jsx +++ b/resources/js/components/ChatBox.jsx @@ -5,6 +5,9 @@ import OnlineUsers from "./OnlineUsers.jsx"; const ChatBox = ({rootUrl, csrfToken}) => { + // todo: interaction with HTML should be put in useEffect hook + // in this case the component will not do actions every render + // put getElementById, JSON.parse, getting authUser and chatChannel to useEffect const chatData = document.getElementById('main') .getAttribute('data-chat'); const chatObject = JSON.parse(chatData); @@ -69,7 +72,8 @@ const ChatBox = ({rootUrl, csrfToken}) => { window.Echo.leave(chatChannel); } }, []); - + // todo: you can use eslinter/prettier in future to make your code more attractive + // indentation in two spaces, attributes in one line etc. return (
diff --git a/resources/js/components/CustomAction.jsx b/resources/js/components/CustomAction.jsx index 83b23af..4f444f4 100644 --- a/resources/js/components/CustomAction.jsx +++ b/resources/js/components/CustomAction.jsx @@ -1,3 +1,4 @@ +// todo: not a class or component, call it customAction const CustomAction = ({action, data}) => { switch (action) { diff --git a/resources/js/components/InviteModal.jsx b/resources/js/components/InviteModal.jsx index 460dce0..abaf406 100644 --- a/resources/js/components/InviteModal.jsx +++ b/resources/js/components/InviteModal.jsx @@ -3,6 +3,8 @@ import SendRequest from "./SendRequest.jsx"; const InviteModal = ({rootUrl, csrfToken}) => { + // todo: you can put invitesEndPoint, chatsEndPoint, systemData, system in useEffect and save + // to prevent call it every render const invitesEndPoint = `${rootUrl}/invites`; const chatsEndPoint = `${rootUrl}/chats/`; const systemData = document.getElementById('invite') @@ -38,6 +40,7 @@ const InviteModal = ({rootUrl, csrfToken}) => { chat_id: e.chat.id, _token: csrfToken, }; + // todo: sendRequest maybe await SendRequest({ endPoint: invitesEndPoint, data diff --git a/resources/js/components/Message.jsx b/resources/js/components/Message.jsx index 1e50ee1..09aff68 100644 --- a/resources/js/components/Message.jsx +++ b/resources/js/components/Message.jsx @@ -5,6 +5,8 @@ const Message = ({rootUrl, authUser, message, csrfToken}) => { const messagesEndPoint = `${rootUrl}/messages/`; + // todo: maybe isOwnMessage or + // const isOwnMessage = authUser.id === message.user.id; const ownMessage = () => { return (authUser.id === message.user.id); }; @@ -15,12 +17,15 @@ const Message = ({rootUrl, authUser, message, csrfToken}) => { _token: csrfToken, _method: 'delete' }; + + // todo: sendRequest maybe await SendRequest({ endPoint: deleteMessageEndpoint, data }); } + // todo: method/function's name should represent the action, kinda getAlertType const alert = () => { switch (true) { case authUser.id === message.user_id : diff --git a/resources/js/components/MessageInput.jsx b/resources/js/components/MessageInput.jsx index 0733c56..9a3c1cf 100644 --- a/resources/js/components/MessageInput.jsx +++ b/resources/js/components/MessageInput.jsx @@ -16,6 +16,8 @@ const MessageInput = ({rootUrl, csrfToken, chatObject}) => { chat_id: chat.id, _token: csrfToken, }; + + // todo: maybe sendRequest await SendRequest({ endPoint: messagesEndPoint, data @@ -25,6 +27,7 @@ const MessageInput = ({rootUrl, csrfToken, chatObject}) => { const createMessage = (e) => { e.preventDefault(); if (message.trim() === "") { + // todo: sometimes user has disabled alerts alert("Please enter a message!"); return; } @@ -32,6 +35,8 @@ const MessageInput = ({rootUrl, csrfToken, chatObject}) => { setMessage(""); }; + + // todo: maybe handleMessage or inputHandler const typingText = (e) => { let text = e.target.value; setMessage(text); @@ -43,6 +48,7 @@ const MessageInput = ({rootUrl, csrfToken, chatObject}) => { data: authUser }); } + // todo: maybe broadcastEvent BroadcastEvent({ channel, event: 'typing', diff --git a/resources/js/components/Online.jsx b/resources/js/components/Online.jsx index cd14b4e..6c64447 100644 --- a/resources/js/components/Online.jsx +++ b/resources/js/components/Online.jsx @@ -17,6 +17,8 @@ const Online = ({user, chatObject}) => { /** * Block sending invite to yourself */ + + // todo: maybe inviteUser, comment is not needed here because this is obvious from the code const dontInviteYourself = () => { if (user.id !== authUser.id) { return ( diff --git a/resources/js/components/OnlineUsers.jsx b/resources/js/components/OnlineUsers.jsx index 8b4d76c..de6e170 100644 --- a/resources/js/components/OnlineUsers.jsx +++ b/resources/js/components/OnlineUsers.jsx @@ -21,6 +21,7 @@ const OnlineUsers = ({chatObject}) => { }; const updateStatusBar = (action, data) => { + // todo: maybe customAction let text = CustomAction({action, data}); setStatusBar(text); console.log(text); @@ -85,6 +86,7 @@ const OnlineUsers = ({chatObject}) => { window.Echo.private(systemChannel) .listen('.ChatCreated', (e) => { const chatCreateSB = () => (updateStatusBar('chatCreate', e.model)); + // todo: here you can save setTimeout's id and use clearTimeout(id) in return of useEffect setTimeout(chatCreateSB, 2000); }) .listen('.ChatDeleted', (e) => { @@ -101,6 +103,7 @@ const OnlineUsers = ({chatObject}) => { connectChatChannel() connectPrivateSystemChannel(); return () => { + // todo: remove setTimeout's id window.Echo.leave(channel); window.Echo.leave(systemChannel); } diff --git a/resources/js/components/SendRequest.jsx b/resources/js/components/SendRequest.jsx index b91d032..0018e34 100644 --- a/resources/js/components/SendRequest.jsx +++ b/resources/js/components/SendRequest.jsx @@ -1,5 +1,8 @@ +// todo: sendRequest maybe const SendRequest = async ({endPoint, data}) => { try { + // todo: axios should be imported from the file with all needed configuration + // not good idea to put it in global object await axios.post(endPoint, data); } catch (error) { console.log(error.message); diff --git a/resources/js/echo.js b/resources/js/echo.js index 61aeddb..e5654c6 100644 --- a/resources/js/echo.js +++ b/resources/js/echo.js @@ -1,7 +1,7 @@ import Echo from 'laravel-echo'; import Pusher from 'pusher-js'; - +// todo: don't put objects in global scope window.Pusher = Pusher; window.Echo = new Echo({