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

Feat/show register on dom #43

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Conversation

RitaCaterina
Copy link
Collaborator

No description provided.

Rita and others added 10 commits December 18, 2023 11:28
new file mode 100644
index 0000000..cbd4e01
--- /dev/null
+++ b/.vscode/settings.json
@@ -0,0 +1,7 @@
+{
+    "workbench.colorCustomizations": {
+        "activityBar.background": "#243210",
+        "titleBar.activeBackground": "#324617",
+        "titleBar.activeForeground": "#F7FBF2"
+    }
+}
\ No newline at end of file
Copy link
Collaborator

@Rizaaal Rizaaal left a comment

Choose a reason for hiding this comment

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

credo che il commit sia basato su un main indietro rispetto ai commit più recenti

Copy link
Collaborator

Choose a reason for hiding this comment

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

Credo che queste siano i settaggi di vscode, e non dovrebbero stare in un commit. Credo sia sufficiente rimuovere il file dal commit per ora, ma evidentemente c'è un problema con il .gitignore?

Rita and others added 2 commits January 18, 2024 18:08
@Rizaaal Rizaaal force-pushed the feat/show-register-on-dom branch from e643444 to bb7c2f9 Compare January 18, 2024 17:10
i added an <aside> 2 times by mistake
Copy link
Owner

@Aierbote Aierbote left a comment

Choose a reason for hiding this comment

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

The 2 closing tag </div> in registri.html line 92, 93 distrupts the normal flow of cointainer which had a footer anchored at the bottom

Copy link
Collaborator

@Rizaaal Rizaaal left a comment

Choose a reason for hiding this comment

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

Ecco un altra review

Copy link
Collaborator

Choose a reason for hiding this comment

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

Il problema è che il container per i registri esisteva già:
riga 72: <div class="Registro flex-grow-1"></div>

@@ -2,6 +2,9 @@

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

qui c'è un tag head sdoppiato, qualcosa sarà andato storto nel merge

@@ -24,7 +27,7 @@
crossorigin="anonymous"
></script>
<script src="script.js"></script>
<script type="module" src="view.js" defer></script>
<script src="view.js" defer></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perchè è stata rimossa la dichiarazione module?

<button class="btn btn-primary BottoneMostraLezioni">
Lezioni
<button class="BottoneMostraLezioni">
<a class="Lezioni">Lezioni</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

merge errati da 67 a 72, andavano lasciati i bottoni

script.js Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Va bene, ma va lasciata la parte della CRUD degli studenti che utilizza il localstorage

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.

3 participants