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

Update class.AttachmentPreviewPlugin.php #40

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leandrovergara
Copy link

Added on elseif from line 1059 an extra validation in order to use this plugin in task issues.

Added on elseif from line 1059 an extra validation in order to use this plugin in task issues.
@ilippert
Copy link

ilippert commented Aug 1, 2019

@leandrovergara, in my osticket, v.1.12 if I implement your change, I cannot access osticket anymore.

@leandrovergara
Copy link
Author

Can you send me the error that you are having? Which version of AttachmentPreviewPlugin are you using?

@leandrovergara
Copy link
Author

leandrovergara commented Aug 5, 2019 via email

@ilippert
Copy link

ilippert commented Aug 5, 2019

Dear Leandro,

thanks for writing.

I used the plugin from https://codeload.github.com/clonemeagain/attachment_preview/zip/master

I did not experience an error beyond simply nothing showing at all. the apache log tells me

PHP Parse error: syntax error, unexpected '||' (T_BOOLEAN_OR)

The lines in the code look like this

  elseif (strpos($url, 'index.php') !== FALSE ||
  strpos($url, 'tickets.php') !== FALSE) ||
  strpos($url, 'tasks.php') !== FALSE) {
  // Might be a ticket or a task page.
  $tickets_view = TRUE;

Cheers, Ingmar

@@ -1057,8 +1057,9 @@ public static function isTicketsView() {
$tickets_view = FALSE;
}
elseif (strpos($url, 'index.php') !== FALSE ||
strpos($url, 'tickets.php') !== FALSE) {
// Might be a ticket page..
strpos($url, 'tickets.php') !== FALSE) ||
Copy link
Owner

Choose a reason for hiding this comment

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

Syntax error with this brace ), suggest removing it.

@ilippert
Copy link

ilippert commented Aug 6, 2019

whilst this suggested change does achieve not breaking the site, it does not yet achieve resulting in the equivalent collapsible boxes within the task views, as within the tickets.

@leandrovergara
Copy link
Author

Change have been commited on branch. Please check again an tell me please.

Copy link

@ilippert ilippert left a comment

Choose a reason for hiding this comment

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

as mentioned earlier, this change does not break the system. however, it does not yet achieve actually showing the collapsible frames for the attachment within the task view. That is, the "Show Attachment" button is not shown.

@leandrovergara
Copy link
Author

I don't use "Show Attachment" functionality, because most of attachment are pdf or images files, so i preview them on task inside the thread. Can you show me some example with a screenshot so i could help your issue?

image

@ilippert
Copy link

ilippert commented Aug 6, 2019

in a ticket view, I get this result:
https://ibb.co/4Kb9G7t

in a task I get this result:
https://ibb.co/8cczvSc

@clonemeagain
Copy link
Owner

We don't use tasks, but with the recent PR merge, this conflicts, so needs a touch of rework, apologies

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