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

Various improvements #116

Merged
merged 6 commits into from
Jul 27, 2021
Merged

Various improvements #116

merged 6 commits into from
Jul 27, 2021

Conversation

udan11
Copy link
Contributor

@udan11 udan11 commented Jul 24, 2021

This PR contains various performance improvements and uploads-related bug fixes. See each individual commit.

It used to fetch mentions, hashtags and uploads for each post which
caused a lot of API calls (one per entity type per post). This reduces
the number of API calls to one per entity.
@udan11 udan11 requested a review from ZogStriP July 24, 2021 11:11
@udan11 udan11 force-pushed the develop branch 5 times, most recently from ac6fd9a to 9c0c956 Compare July 26, 2021 16:40
@udan11 udan11 requested a review from eviltrout July 26, 2021 16:41
Copy link
Contributor

@eviltrout eviltrout left a comment

Choose a reason for hiding this comment

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

I love this commit. It really brings us up to the newer Ember standards.

@@ -31,10 +33,10 @@ export default Ember.Controller.extend(ModalFunctionality, {

actions: {
copy() {
const $copyRange = $("pre.exported-key-pair");
if (copyText("", $copyRange[0])) {
const copyRange = document.querySelector("pre.exported-key-pair");
Copy link
Contributor

Choose a reason for hiding this comment

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

(I realize this didn't change here but we might want a different way to export these in the future that doesn't rely on selecting the DOM.)

const iv = download.buffer.slice(0, 12);
const content = download.buffer.slice(12);

return new Promise((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious about the need to do new Promise here when window.crypto.subtle.decrypt seems to return a promise? Could you not return that?

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 can, but the tests might not work anymore. The runloop is aware of Ember Promises, but is not of the native ones:

https://discuss.emberjs.com/t/readers-questions-why-does-ember-still-use-rsvp/14736

assets/javascripts/lib/uploads.js Outdated Show resolved Hide resolved
Encrypted image uploads were displayed only in post body. This commit
fixes the bug which enlarged small uploaded images.
This commit includes a fix to the problem of same topic title being
displayed to every encrypted topic. The root cause was that jQuery's
`data` method did not return the value of `data-topic-id` attribute.
@udan11 udan11 merged commit 8b995f8 into main Jul 27, 2021
@udan11 udan11 deleted the develop branch July 27, 2021 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants