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 lightbox to timeline images with PhotoSwipe #5170

Merged
merged 1 commit into from Sep 24, 2019

Conversation

cconard96
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #3139

closes #3139

Currently uses single-image galleries in timelines of ITIL Objects. Looking into possibly grouping images and documents in the timeline, but seems like a big task.
UserEcho for document groups idea

@cedric-anne cedric-anne changed the base branch from master to 9.5/bugfixes September 16, 2019 10:53
@cedric-anne
Copy link
Member

Hi @cconard96 ,

I rebased your branch on 9.5/bugfixes, and add some commits to:

  • fetch PhotoSwipe using npm (instead of pushing lib files to our repository),
  • fix rendering of thumbnail (I put back exisiting styles),
  • clean some code that seems useless.

I did not reviewed all code yet, but I will do it in order to put this feature into 9.5/bugfixes (if other core devs are OK for this).

Regards

Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

OK for me.

I added some commits, so do not hesitate to tell me if I have done something wrong.

@cedric-anne cedric-anne added this to the 9.5.0 milestone Sep 16, 2019
@cconard96
Copy link
Contributor Author

Good for me with one minor change. The figures were incorrectly sized so clicking to the right of the thumbnail would still trigger the lightbox.

trasher
trasher previously approved these changes Sep 17, 2019
@orthagh
Copy link
Contributor

orthagh commented Sep 17, 2019

One issue, it seems to be ok only for attached documents.
For inline pictures (in followups/task/etc), they still open in a separate tab.

It's disturbing on my side, because we have different behaviors for same objets (but with different contexts)

@cconard96
Copy link
Contributor Author

I can't get the images to open at all when embedded in followups or tasks. It is just a static img element. Am I missing something? Adding as a document or attachment (under a followup or task) seems to work OK.

@cedric-anne
Copy link
Member

One issue, it seems to be ok only for attached documents.
For inline pictures (in followups/task/etc), they still open in a separate tab.

It's disturbing on my side, because we have different behaviors for same objets (but with different contexts)

I can't get the images to open at all when embedded in followups or tasks. It is just a static img element. Am I missing something? Adding as a document or attachment (under a followup or task) seems to work OK.

I added a commit to handle this.

inc/html.class.php Outdated Show resolved Hide resolved
@trasher trasher dismissed their stale review September 24, 2019 07:14

PR has changed

Copy link
Contributor

@trasher trasher left a comment

Choose a reason for hiding this comment

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

OK for me :)

@trasher trasher merged commit bbba93b into glpi-project:9.5/bugfixes Sep 24, 2019
@cconard96 cconard96 deleted the feature/lightbox branch September 24, 2019 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants