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

Button to display message source in the New Modmail #336

Merged
merged 18 commits into from
Aug 20, 2020

Conversation

xeoth
Copy link
Contributor

@xeoth xeoth commented Jul 2, 2020

If I did everything right, this fixes #323

Copy link
Member

@creesch creesch left a comment

Choose a reason for hiding this comment

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

Had a quick look with some initial comments. Will go over it in more detail when I have a bit more time.


// Fetch and store the conversation info in cache
const currentID = event.detail.pageDetails.conversationID;
TBStorage.setCache('NewModmailPro', 'current-conversation', await TBApi.apiOauthGET(`/api/mod/conversations/${currentID}`).then(r => r.json()));
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure we need to do the request for every modmail loaded. Possibly just do the request the first time source is clicked

Copy link
Member

Choose a reason for hiding this comment

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

You haven't actually resolved this, for every message in a conversation you are still doing a request.

My suggestion would be to set up your cache something along these lines:

  1. Keep a local boolean named conversationCached and set it to false by default.
  2. Attach the buttons to the replies.
  3. When a source button is clicked check the boolean, if true use cache for the source if false fetch the conversation and set the cache.
  4. Clear the boolean based on a TBNewPage event.

extension/data/modules/newmodmailpro.js Outdated Show resolved Hide resolved
@eritbh eritbh self-requested a review July 9, 2020 19:22
@xeoth xeoth requested a review from creesch August 14, 2020 14:08
@xeoth
Copy link
Contributor Author

xeoth commented Aug 14, 2020

Thanks for your comments, Creesch. I wouldn't have noticed those. I fixed what needed fixing and added a toggling feature. It should all be good now.

Please review and leave a comment if you find something that still needs a fix.

Copy link
Member

@creesch creesch left a comment

Choose a reason for hiding this comment

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

So uh yeah I ended up testing my suggested changes to make sure that what I was saying makes sense.

So if you want you can simply use the below code ;)

        if (sourceButton) {
            let conversationCached = false;
            window.addEventListener('TBNewPage', () => {
                conversationCached = false;
            });

            TB.listener.on('author', event => {
                if (event.detail.type !== 'TBmodmailCommentAuthor') {
                    return;
                }
                const $target = $(event.target);

                $target.closest('.Message__header').append('<button class="tb-source-button tb-general-button">source</button>');

                $('.tb-source-button').click(async e => {
                    // Something is causing the listener to be triggered multiple times.
                    e.stopImmediatePropagation();
                    const $currentSourceBtn = $(e.currentTarget.parentElement);

                    // Getting the ID of the message on which the button was clicked.
                    const activeMessageID = event.detail.data.comment.id;
                    const $currentSourceField = $(`#tb-source-${activeMessageID}`);

                    // Toggling the source
                    if ($currentSourceField.length) {
                        // If the source field exists, toggle it

                        $currentSourceField.toggle();
                    } else {
                        // If the source field is not present (has not been requested yet), request it and create
                        // a div+textarea with the source.

                        if (!$currentSourceBtn.closest('.Thread__message').has('.tb-source-field').length) {
                            let conversationInfo;
                            if (conversationCached) {
                                conversationInfo = await TBStorage.getCache('NewModmailPro', 'current-conversation');
                            } else {
                                // Fetch and store the conversation info in cache
                                const currentID = event.detail.data.post.id;
                                conversationInfo = await TBApi.apiOauthGET(`/api/mod/conversations/${currentID}`).then(r => r.json());
                                TBStorage.setCache('NewModmailPro', 'current-conversation', conversationInfo);
                                conversationCached = true;
                            }
                            // Getting the body in markdown from selected message
                            const messageSource = conversationInfo.messages[activeMessageID].bodyMarkdown;

                            $currentSourceBtn.closest('.Thread__message').append(`
                                <div class="tb-source-field" id="tb-source-${activeMessageID}">
                                    <textarea readonly></textarea>
                                </div>`);
                            $(`#tb-source-${activeMessageID} textarea`).text(messageSource);
                        }
                    }
                });
            });
        }

extension/data/modules/newmodmailpro.js Outdated Show resolved Hide resolved

// Fetch and store the conversation info in cache
const currentID = event.detail.pageDetails.conversationID;
TBStorage.setCache('NewModmailPro', 'current-conversation', await TBApi.apiOauthGET(`/api/mod/conversations/${currentID}`).then(r => r.json()));
Copy link
Member

Choose a reason for hiding this comment

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

You haven't actually resolved this, for every message in a conversation you are still doing a request.

My suggestion would be to set up your cache something along these lines:

  1. Keep a local boolean named conversationCached and set it to false by default.
  2. Attach the buttons to the replies.
  3. When a source button is clicked check the boolean, if true use cache for the source if false fetch the conversation and set the cache.
  4. Clear the boolean based on a TBNewPage event.

extension/data/modules/newmodmailpro.js Outdated Show resolved Hide resolved
extension/data/modules/newmodmailpro.js Outdated Show resolved Hide resolved
extension/data/styles/newmodmailpro.css Show resolved Hide resolved
@creesch creesch merged commit 47ccf57 into toolbox-team:master Aug 20, 2020
eritbh pushed a commit that referenced this pull request Sep 9, 2020
* Source button looks like other ones

* Source button activates on every modmail thread

* displaying source with textarea works

* resized the field

* removed styling for the button since it got converted to an action button

* prevented displaying multiple source textareas

* moved button below the message, simplified code

* moved button down to not collide with the message

* fixed style issues

* Added a setting

* Added toggling source

* Changed some comments

* Refactor: Moved to TB.listener and button appears only once

* Changed the textarea size

* Simplified hiding/showing the textarea

* Source appears only once

* Got rid of a needless comment

* Applied @creesch's changes

Co-authored-by: Xeoth <ujan.pl@gmail.com>
eritbh pushed a commit that referenced this pull request Sep 5, 2024
* Source button looks like other ones

* Source button activates on every modmail thread

* displaying source with textarea works

* resized the field

* removed styling for the button since it got converted to an action button

* prevented displaying multiple source textareas

* moved button below the message, simplified code

* moved button down to not collide with the message

* fixed style issues

* Added a setting

* Added toggling source

* Changed some comments

* Refactor: Moved to TB.listener and button appears only once

* Changed the textarea size

* Simplified hiding/showing the textarea

* Source appears only once

* Got rid of a needless comment

* Applied @creesch's changes

Co-authored-by: Xeoth <ujan.pl@gmail.com>
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.

Add "source" button to new modmail
2 participants