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

Fix archive conversation issues #732

Merged
merged 19 commits into from
Jan 20, 2019
Merged

Conversation

dusansimic
Copy link
Collaborator

Fixed archive conversation issues. Now when a conversation is archived, it automatically switches to the next conversation so there wont be an empty display. Also with this PR fixed the issue with archiving group conversations.

Closes: #590
Closes: #591

When a chat is archived the selection is switched to the next one. Also
added some fixes using `elementReady()` since some elements are
requested before page is loaded.
Also fixed leaving group conversations issue.
Copy link
Collaborator

@CvX CvX left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the PR! I've added feedback about stuff that's I've noticed is broken or could be broken for other users.

browser.js Show resolved Hide resolved
browser.js Outdated Show resolved Hide resolved
browser.js Outdated Show resolved Hide resolved
Non-async functions that call async functions are now turned to async.
Fixed that check for chat type via chat options menu. Also explained the
process in the comment.
Reverting to use of elementReady for getting element.
@dusansimic
Copy link
Collaborator Author

@CvX here are updates. This should be it.
Regarding that openPreferences function, I think we should make showSettingsMenu function async so it uses elementReady instead of document.querySelector.

@CvX
Copy link
Collaborator

CvX commented Jan 18, 2019

I like the idea of making the showSettingsMenu async. 🙂 Could you do it in this PR?

showSettingsMenu is used in a cuple of places and in one place it needs
to wait for element to be created to show the menu. In that place so far
we've been using a custom function elementReady instead of
document.querySelector. Now showSettingsMenu is made async so it can now
use elementReady.
@dusansimic
Copy link
Collaborator Author

Here it is 😃

dusansimic and others added 11 commits January 19, 2019 04:16
Here's a work around. It's ugly, it's not foolproof but it works.
Merged the branch with CvX's arvhice-fix branch that contains new
mechanism for querying menus and clicking on menu items. Also used that
new mechanism for openArchiveModal function.

Great work by @CvX.
Since that’s what this function does. There’s no modal.
That argument is 1-based, it later gets converted to a 0-based index.
Prevents errors in situations where conversation list is replaced with another view.
@CvX
Copy link
Collaborator

CvX commented Jan 19, 2019

I've cleaned up the code a little bit. Please take a look, I want to be sure everything is as expected. 😉

@dusansimic
Copy link
Collaborator Author

dusansimic commented Jan 19, 2019

Looks good 👌🏻. I've just tested it and found out that actually delete chat option behaves just like archive option used to, gets removed from the list and leaves the chat panel empty. Should we add the same feature (open next conversation) to delete chat option while were here?

@CvX
Copy link
Collaborator

CvX commented Jan 19, 2019

Oh, yeah. 😅That would be great! 😃

Function `openDeleteModal` was renamed to `deleteSelectedConverastion`
and is now after delete selecting next conversation so chat panel wont
be empty.
@sindresorhus sindresorhus merged commit 5c457c7 into sindresorhus:master Jan 20, 2019
sindresorhus pushed a commit that referenced this pull request Jan 20, 2019
@dusansimic dusansimic deleted the fix-591 branch January 20, 2019 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants