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

Add edit functionality for notes #101

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/db.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ class App < Sinatra::Base
end
end

put "/db/notes/:id" do
@record = Ronin::DB::Note.find_by(params[:id])

unless @record || @record.update(params)
Copy link
Member

Choose a reason for hiding this comment

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

I would return a different error code if the update fails, like 400 (Bad Request).

halt 404
end
end

get '/db/asns' do
@pagy, @asns = pagy(Ronin::DB::ASN)

Expand Down
15 changes: 15 additions & 0 deletions public/javascript/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,19 @@ ready(() => {
}
});
});

(document.querySelectorAll('.dropdown') || []).forEach(dropdown => {
dropdown.addEventListener('click', function(event) {
event.stopPropagation();
dropdown.classList.toggle('is-active');
});
})

document.addEventListener('click', function (event) {
(document.querySelectorAll('.dropdown') || []).forEach(dropdown => {
if (!dropdown.contains(event.target)) {
dropdown.classList.remove('is-active')
}
})
})
});
47 changes: 47 additions & 0 deletions public/javascript/notes.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,21 @@ const Notes = {
Notes.delete(noteId, button)
});
});

(document.querySelectorAll('.edit-note') || []).forEach(button => {
button.addEventListener('click', () => {
const noteId = button.getAttribute('data-note-id')
Notes.toggleEditForm(noteId)
})
});

(document.querySelectorAll('.update-note') || []).forEach(button => {
button.addEventListener('click', () => {
const noteId = button.getAttribute('data-note-id')
Notes.toggleEditForm(noteId)
Notes.update(noteId)
})
});
},

delete(noteId, button) {
Expand All @@ -22,6 +37,38 @@ const Notes = {
.catch(error => {
console.error('Error:', error);
});
},

toggleEditForm(noteId) {
const editForm = document.getElementById(`note-edit-form-${noteId}`)
const body = document.getElementById(`note-edit-body-${noteId}`)

if (editForm.style.display === 'none') {
body.style.display = 'none';
editForm.style.display = 'block';
} else {
body.style.display = 'block';
editForm.style.display = 'none';
}
},

update(noteId) {
const body = document.getElementById(`note-edit-body-${noteId}`)
const newValue = document.getElementById(`note-edit-textarea-${noteId}`).value

fetch(`${document.location.origin}/db/notes/${noteId}?` + new URLSearchParams({ body: newValue }), {
method: 'PUT'
})
.then(response => {
if (response.ok) {
body.textContent = newValue
} else {
console.error('Failed update a note');
}
})
.catch(error => {
console.error('Error:', error);
});
}
};

Expand Down
28 changes: 28 additions & 0 deletions public/stylesheets/app.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
--fg-color: black;
--dark-mode-bg-color: #0f0f1b;
--dark-mode-fg-color: white;
--dark-mode-secondary-bg-color: #141425;
}

body {
Expand Down Expand Up @@ -175,3 +176,30 @@ body.dark-mode main a {
padding-top: 0;
border-top: 0;
}

body.dark-mode .dropdown-content {
background-color: var(--dark-mode-secondary-bg-color);
}

.dropdown-item {
text-decoration: none !important;
padding: 8px 32px !important;
text-align: center !important;
}

.dropdown-item.delete-note:hover {
background-color: hsl(348, 100%, 61%) !important;
color: var(--dark-mode-fg-color) !important;
}

.dropdown-menu {
min-width: 0 !important;
}

.dropdown.three-dots {
cursor: pointer;
}

.dropdown.three-dots:hover {
color: rgb(105, 105, 198) !important;
}
52 changes: 43 additions & 9 deletions views/_notes.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,53 @@

<% unless notes.empty? %>
<h3>Notes</h3>
<% notes.each do |note| %>
<div class="box">
<div class="columns is-full">
<div class="column">
<h4 class="mb-0"><%= note.body %></h4>
<small>Created at: <%= note.created_at %></small>

<% notes.each do |note| %>
<div class="box p-0" style="border: 1px solid #1c1c39">
<div class="columns m-0 p-1" style="background-color: #1c1c39">
<div class="column is-half px-3 py-0">
<small class="is-size-7">Created at: <%= note.created_at %></small>
Copy link
Member

Choose a reason for hiding this comment

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

Probably safe, but I'd escape the timestamp using h just to be safe.

</div>

<div class="column has-text-right p-0 pr-1 mt-0">
<div class="dropdown three-dots is-right">
<div class="dropdown-trigger">
<span class="icon is-small">
<i class="fas fa-ellipsis-h"></i>
</span>
</div>
<div class="dropdown-menu" id="dropdown-menu6" role="menu">
<div class="dropdown-content">
<a class="dropdown-item py-2 edit-note" data-note-id=<%= note.id %>>Edit</a>
Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea to escape note.id with hattr just to be safe, and that it doesn't trigger false positives on some security scan.

<a class="dropdown-item py-2 delete-note has-text-danger" data-note-id=<%= note.id %>>Delete</a>
Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea to escape note.id with hattr just to be safe, and that it doesn't trigger false positives on some security scan.

</div>
</div>
</div>
<div class="column is-one-fifth has-text-right">
<button class="button is-danger is-small delete-note " data-note-id=<%= note.id %>>X</button>
</div>
</div>

<div class="columns is-full">
<div class="column">
<div id="note-edit-body-<%= note.id %>" class="mb-0 p-4"><%= note.body %></div>
Copy link
Member

Choose a reason for hiding this comment

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

Needs h HTML escaping on note.body, or we should render it as markdown.

Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea to escape note.id with hattr just to be safe, and that it doesn't trigger false positives on some security scan.


<div id="note-edit-form-<%= note.id %>" style="display: none">
Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea to escape note.id with hattr just to be safe, and that it doesn't trigger false positives on some security scan.

<div class="control">
<div class="media-content">
<div class="field mb-0 p-2">
<textarea id="note-edit-textarea-<%= note.id %>" class="textarea" name="body" placeholder="Edit a note..."><%= note.body %></textarea>
Copy link
Member

Choose a reason for hiding this comment

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

Needs h HTML escaping, or we need to render the note body as markdown and inline the primitive HTML.

Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea to escape note.id with hattr just to be safe, and that it doesn't trigger false positives on some security scan.

</div>

<div class="field is-flex is-justify-content-flex-end px-2 pb-2">
<button class="edit-note button is-small is-danger mr-2" data-note-id=<%= note.id %>>Discard</button>
<button class="update-note button is-small is-primary" data-note-id=<%= note.id %> data-note-body=<%= note.body %>>Save</button>
Copy link
Member

Choose a reason for hiding this comment

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

Attributes need hattr escaping.

</div>
</div>
</div>
</div>
</div>
</div>
<% end %>
</div>
<% end %>
<% end %>

<div class="control mt-4">
Expand Down
1 change: 1 addition & 0 deletions views/layout.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<link rel="stylesheet" type="text/css" media="screen" href="/stylesheets/bulma.min.css" />
<link rel="stylesheet" type="text/css" media="screen" href="/stylesheets/app.css" />
<script type="text/javascript" src="/javascript/app.js"></script>
<script defer src="https://use.fontawesome.com/releases/v5.3.1/js/all.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Let's not add a font just yet and use the browser's default font, especially not some external JavaScript asset font. The whole app should be able to load and run without an internet connection.

</head>

<body>
Expand Down