London | 26-ITP-Jan | Asha Ahmed | Sprint 2 | Book Library#421
London | 26-ITP-Jan | Asha Ahmed | Sprint 2 | Book Library#421ashaahmed7 wants to merge 2 commits intoCodeYourFuture:mainfrom
Conversation
ashaahmed7
added
the
Needs Review
Luro91
added
Review in progress
Luro91
left a comment
Luro91
left a comment
There was a problem hiding this comment.
How should I run the project?
When I open the html file in the browser I get this error:
index.html:1 Access to script at Module-Data-Flows/debugging/book-library/script.js' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: chrome, chrome-extension, chrome-untrusted, data, http, https, isolated-app. script.js:1 Failed to load resource: net::ERR_FAILED index.html:1 Unsafe attempt to load URL file://Module-Data-Flows/debugging/book-library/index.html from frame with URL file:///Module-Data-Flows/debugging/book-library/index.html. 'file:' URLs are treated as unique security origins.
| .form-group { | ||
| width: 400px; | ||
| height: 300px; | ||
| align-self: left; |
There was a problem hiding this comment.
Why did you change this?
THe commit messages "Refactor input validation and improve book submission logic in script.js" does not make this clear to me
There was a problem hiding this comment.
It looked a bit off to me to I decided to change it.
I will be more careful when I commit a file and make sure it doesn't get conflicted with my commit message in the near future, sorry for the confusion.
There was a problem hiding this comment.
I see. It's good practice to split up commits into logical parts and not add all changes in one commit:
A commit should describe what changed in the code. It's also good practice to split commits into logical blocks (for example for each task or each review comment). This makes it easier to understand and additionally the commits could also be undone more easily if needed.
I like this article about commit messages: https://chris.beams.io/git-commit
| pagesCount <= 0 | ||
| ) { | ||
| alert("Please fill all fields!"); | ||
| alert( |
Luro91
added
Reviewed
I've just tested this. I right clicked on html file and selected live server and it took me straight to my book library. |
ashaahmed7
added
Needs Review
| </table> | ||
|
|
||
| <script src="script.js"></script> | ||
| <script src="script.js" type="module"></script> |
There was a problem hiding this comment.
Why did you change this to module? (It is the reason why the app only works in the live server and not if you just open the html file in the browser directly)
Luro91
added
Complete
|
Well done. The code works as expected. The only comment I have is on the change to module. |
2 participants
Learners, PR Template
Self checklist
Changelist
I fixed the bugs in the code and corrected the syntax
Questions
N/A