added drag and drop icon to metadata panel. Also there are some space fixes. #566

aeschylus · 2016-05-18T21:00:15Z

connected to #622

houndci-bot · 2016-05-18T21:00:28Z

lib/embed/viewer/image_x.rb

@@ -8,8 +8,8 @@ def initialize(*args)

      def body_html
        Nokogiri::HTML::Builder.new do |doc|
-          doc.div(class: 'sul-embed-body sul-embed-image-x', 'data-sul-embed-theme' => asset_url('image_x.css').to_s) do
-            doc.div(id: 'sul-embed-image-x', 'data-manifest-url' => manifest_json_url, 'data-world-restriction' => !@purl_object.world_unrestricted?) {}
+          doc.div(class: 'sul-embed-body sul-embed-image-x', 'data-sul-embed-theme' => "#{asset_url('image_x.css')}") do


Prefer to_s over string interpolation.

aeschylus · 2016-05-18T21:03:12Z

This is a PR for the purposes of design review. See the gifs below:

coveralls · 2016-05-18T21:06:03Z

Coverage remained the same at 98.371% when pulling ae0f0ecb18cc5500d50d1979d257f7868ccbf9d2 on add_iiif_drag_and_drop into 648cfaf on master.

aeschylus · 2016-05-18T21:24:13Z

There are probably some more styling updates we want to make. The underline shouldn't appear underneath the icon itself, and we should perhaps adjust the whitespace and text alignment of the instructions with the icon.

aeschylus · 2016-05-18T21:35:46Z

see also: #567

snydman · 2016-05-18T23:51:05Z

@mejackreed @jvine @ggeisler can you please review this and give feedback? I'd like the media team to get this in if possible during this wave of work on sul-embed.

ggeisler · 2016-05-19T00:12:29Z

Should there be an underline on any part of this? The text is not a link, correct?
Can the icon be a tiny bit smaller? It feels a little out of proportion to me (although it is a bit tricky to judge, having to watch the gif repeat over and over; the gif is nice for showing the interaction but a still image might make it easier to review this for design purposes).
I wonder if we can think of better wording for the action? It's a little odd because it is under a heading called "Available Online" but it's not really an availability link like the PURL link. Also, the instruction text doesn't tell the user what it is for (why are you telling me to drag the icon?), just how to do the action.

Maybe something like:

View image in a IIIF viewer (drag icon to another viewer)

Or maybe just:

View image in a IIIF viewer (drag icon)

?

jvine · 2016-05-19T00:28:43Z

Huh, I'm wondering where the "Available Online" header came from - I hadn't noticed it before, and it doesn't seem appropriate here at all. I think we were referring to the PURL as the "Citation URL", but that doesn't provide a good label for the IIIF logo.

I don't think I mind the size of the logo - a larger mouse target for a drag operation doesn't seem inappropriate - though it's hard to tell with the small screenshot.

I agree that the current text doesn't seem like it should be a link. But at one point we were talking about linking to an external page that explains about IIIF and viewers. Does such a page exist? And if so, should we link to it?

ggeisler · 2016-05-19T00:39:57Z

If there is a linkable reference page, maybe a variation of my previous suggestion:

Drag icon to view image in a IIIF viewer (more information)

Where "more information" is the link? (and is underlined as a link)

snydman · 2016-05-19T05:23:33Z

Alas there is not such an informational page to link to, unless we go to http://iiif.io, but that probably wouldn't be too helpful. I guess we should create a page...

aeschylus · 2016-05-19T18:11:58Z

aeschylus · 2016-05-19T18:36:38Z

@ggeisler Good point about the underline. I'm not sure what I was thinking. The item is an anchor with an href in the markup, and that forces it to have an underline, but I never overrode it it seems. I agree it should be removed, and that the icon should be roughly the same size as the text, with the vertical alignment made less janky.

I added some tweaks above. Let me know if any of those seem right and I will update the PR.

jvine · 2016-05-19T18:51:06Z

Why is it an anchor with an href? The icon itself needs to be focusable, but the text doesn't.

I prefer the original text with the action closest to the thing it's acting on: Drag icon to view... @ggeisler?

And any other thoughts for the heading? Or is that out of scope of this PR?

ggeisler · 2016-05-19T18:57:40Z

Thanks for the updates @aeschylus. I like the third version, but @jvine has done more work on this so I'd defer to her. I will say I much prefer the text in the third version rather than the original text, though. What percentage of users will even know what IIIF is? Telling them to drag the icon without explaining what the purpose of dragging it is seems odd to me.

If we really want to start the text with the action, maybe something like:

Drag this icon to view image in a IIIF viewer

jvine · 2016-05-19T19:05:39Z

Drag this icon to view image in a IIIF viewer

👍

I agree with the "IIIF viewer" part - I just don't like the "drag icon" to be far away from what it refers to.

mejackreed · 2016-05-19T19:22:49Z

Maybe a cursor: move would help?

aeschylus · 2016-05-19T22:29:22Z

I agree we should add some cursor styling, but I would cast a vote for the open grabby hand (cursor:grab), which also has a "dragging" ("grabbing") style.

https://developer.mozilla.org/en-US/docs/Web/CSS/cursor

@jvine: Should we change the title/sectioning?

I agree there should be some kind of informational link. The closest information page we have is zimeon's personal page on the topic, which contains a number of outdated links. There has been talk of updating this and making it a permanent part of iiif.io.

snydman · 2016-05-23T21:16:02Z

@aeschylus @mejackreed we should produce a new version of a page and do pull request to iiif.io. But it should be different from zimeon's page and oriented towards informing users of what it is and how they can use it. Maybe with a little animated gif or something. We should NOT try to solve the "explaining IIIF" to the masses problem with this, but put something out there that is reasonable and helpful. If the IIIF eds and coco don't accept the page then we can add it to library.stanford.edu for now and change the link later.

jvine · 2016-06-01T23:40:05Z

IIIF feature and PURL are in separate dt/dd pairs
dt for PURL changes to "Citation URL"
dt for icon is "International Image Interoperability Framework"
icon is about 25px wide w 5px right margin
icon has "grab" cursor
instruction text is "Drag icon to open this image/set in a IIIF viewer."
- generic text is okay, but if possible, it would be better to check to see if there is 1 image, or multiple images, and adjust the text accordingly.
- Drag icon to open this image in a IIIF viewer.
- Drag icon to open these images in a IIIF viewer.
text colour for instruction is dimgrey
link on "IIIF viewer" points to http://library.stanford.edu/projects/international-image-interoperability-framework

…space fixes.

…he metadata.

houndci-bot · 2016-08-08T23:09:23Z

app/assets/stylesheets/image_x.scss

+
+  img {
+    width: 25px;
+    margin: 0 5px 0 0 !important;


!important should not be used

houndci-bot · 2016-08-08T23:11:43Z

app/assets/stylesheets/image_x.scss

+  cursor: -webkit-grab;
+
+  &:active {
+    cursor: -webkit-grabbing;


Avoid vendor prefixes.

jkeck · 2016-08-08T23:14:49Z

New progress for review:

zw433vr9951 (single image)

fw090jw3474 (multiple images)

coveralls · 2016-08-08T23:17:19Z

Coverage increased (+0.03%) to 98.521% when pulling 4da8705 on add_iiif_drag_and_drop into cb815fe on master.

jmartin-sul · 2016-08-09T21:24:10Z

👍 from me, but i sort of paired on it with @jkeck yesterday, and so will refrain from being the one to merge

ggeisler · 2016-08-09T21:32:16Z

👍

ndushay · 2016-08-09T21:38:37Z

lib/embed/metadata_panel.rb

    end

    def to_html
      <<-HTML.strip_heredoc
        <div class='sul-embed-panel-container'>
          <div class='sul-embed-panel sul-embed-metadata-panel' style='display:none' aria-hidden='true'>
            <div class='sul-embed-panel-header'>
-              <button class='sul-embed-close' data-sul-embed-toggle='sul-embed-download-panel'>
+              <button class='sul-embed-close' data-sul-embed-toggle='sul-embed-metadata-panel'>


was this an old typo that wasn't caught?

Yeah, copy/paste error I believe.

ndushay · 2016-08-09T21:40:26Z

LGTM aside from a couple of line comments. I only eyeballed it.

snydman · 2016-08-15T17:50:37Z

excited to see this. I do notice that the metadata panel does not appear in PURL versions of the embed. I believe the rationale for this was that it would be duplicative of metadata in the PURL page itself. I suppose this means we ought to add the IIIF logo to the PURL page for images, unless @LynnMcRae and others have a different idea.

jkeck · 2016-08-15T18:05:34Z

This has actually come up elsewhere as well. I believe PURL is now the only app that hides the metadata panel. Do we want to retain this behavior for PURL? (FWIW, we have a version of purl on sul-purl-dev that shows the metadata panel w/ IIIF icon).

jvine · 2016-08-15T18:15:43Z

@ggeisler do you have any opinion about no longer suppressing the info icon, regardless of context? so that the IIIF drag-and-drop is always available. otherwise it needs to be added to specific apps. i'm okay with not suppressing it.

LynnMcRae · 2016-08-15T18:28:27Z

@jkeck seems like SW and Purls are equals here...both show the metadata independently and no harm in SW that it shows some data also in the info panel. I'm fine with that behavior in Purl. It just emphasizes the portability of embed.

ggeisler · 2016-08-16T02:18:16Z

This sounds fine to me, too. I don't see a strong argument for suppressing it.

ndushay added the in progress label May 18, 2016

houndci-bot reviewed May 18, 2016
View reviewed changes

aeschylus changed the title ~~added drag and drop icon to metadata panel. Also there are some white…~~ [DESIGN REVIEW] added drag and drop icon to metadata panel. Also there are some white… May 18, 2016

peetucket modified the milestone: Media Sprint 9 Jul 6, 2016

jvine mentioned this pull request Jul 27, 2016

Add drag and drop IIIF icon to info panel #622

Closed

peetucket changed the title ~~[DESIGN REVIEW] added drag and drop icon to metadata panel. Also there are some white…~~ [DESIGN REVIEW] added drag and drop icon to metadata panel. Also there are some white… Jul 27, 2016

peetucket changed the title ~~[DESIGN REVIEW] added drag and drop icon to metadata panel. Also there are some white…~~ [DESIGN REVIEW] added drag and drop icon to metadata panel. Also there are some space fixes. Jul 27, 2016

aeschylus and others added 2 commits August 8, 2016 13:13

added drag and drop icon to metadata panel. Also there are some white…

5685cbf

…space fixes.

Allow the metadata panel to take content via a block to insert into t…

af539a3

…he metadata.

jkeck force-pushed the add_iiif_drag_and_drop branch from ae0f0ec to 25e83c8 Compare August 8, 2016 23:09

houndci-bot reviewed Aug 8, 2016
View reviewed changes

app/assets/stylesheets/image_x.scss

img {

width: 25px;

margin: 0 5px 0 0 !important;

Copy link

houndci-bot Aug 8, 2016

Choose a reason for hiding this comment

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

!important should not be used

Inject IIIF Drag and Drop icon into metadata panel for image viewer.

4da8705

jkeck force-pushed the add_iiif_drag_and_drop branch from 25e83c8 to 4da8705 Compare August 8, 2016 23:11

houndci-bot reviewed Aug 8, 2016
View reviewed changes

app/assets/stylesheets/image_x.scss

cursor: -webkit-grab;

&:active {

cursor: -webkit-grabbing;

Copy link

houndci-bot Aug 8, 2016

Choose a reason for hiding this comment

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

Avoid vendor prefixes.

jmartin-sul changed the title ~~[DESIGN REVIEW] added drag and drop icon to metadata panel. Also there are some space fixes.~~ added drag and drop icon to metadata panel. Also there are some space fixes. Aug 9, 2016

ndushay reviewed Aug 9, 2016
View reviewed changes

jmartin-sul merged commit ce31233 into master Aug 10, 2016

jmartin-sul deleted the add_iiif_drag_and_drop branch August 10, 2016 00:15

jmartin-sul removed the on deck label Aug 10, 2016

jkeck mentioned this pull request Aug 15, 2016

Remove ability to hide the metadata panel #641

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added drag and drop icon to metadata panel. Also there are some space fixes. #566

added drag and drop icon to metadata panel. Also there are some space fixes. #566

aeschylus commented May 18, 2016 •

edited by peetucket

Loading

houndci-bot May 18, 2016

aeschylus commented May 18, 2016

coveralls commented May 18, 2016

aeschylus commented May 18, 2016

aeschylus commented May 18, 2016

snydman commented May 18, 2016

ggeisler commented May 19, 2016

jvine commented May 19, 2016

ggeisler commented May 19, 2016

snydman commented May 19, 2016

aeschylus commented May 19, 2016 •

edited

Loading

aeschylus commented May 19, 2016

jvine commented May 19, 2016

ggeisler commented May 19, 2016

jvine commented May 19, 2016

mejackreed commented May 19, 2016

aeschylus commented May 19, 2016

snydman commented May 23, 2016

jvine commented Jun 1, 2016

houndci-bot Aug 8, 2016

houndci-bot Aug 8, 2016

jkeck commented Aug 8, 2016

coveralls commented Aug 8, 2016 •

edited

Loading

jmartin-sul commented Aug 9, 2016

ggeisler commented Aug 9, 2016

ndushay Aug 9, 2016

jkeck Aug 9, 2016

ndushay commented Aug 9, 2016

snydman commented Aug 15, 2016

jkeck commented Aug 15, 2016

jvine commented Aug 15, 2016

LynnMcRae commented Aug 15, 2016

ggeisler commented Aug 16, 2016

added drag and drop icon to metadata panel. Also there are some space fixes. #566

added drag and drop icon to metadata panel. Also there are some space fixes. #566

Conversation

aeschylus commented May 18, 2016 • edited by peetucket Loading

houndci-bot May 18, 2016

Choose a reason for hiding this comment

aeschylus commented May 18, 2016

coveralls commented May 18, 2016

aeschylus commented May 18, 2016

aeschylus commented May 18, 2016

snydman commented May 18, 2016

ggeisler commented May 19, 2016

jvine commented May 19, 2016

ggeisler commented May 19, 2016

snydman commented May 19, 2016

aeschylus commented May 19, 2016 • edited Loading

aeschylus commented May 19, 2016

jvine commented May 19, 2016

ggeisler commented May 19, 2016

jvine commented May 19, 2016

mejackreed commented May 19, 2016

aeschylus commented May 19, 2016

snydman commented May 23, 2016

jvine commented Jun 1, 2016

houndci-bot Aug 8, 2016

Choose a reason for hiding this comment

houndci-bot Aug 8, 2016

Choose a reason for hiding this comment

jkeck commented Aug 8, 2016

zw433vr9951 (single image)

fw090jw3474 (multiple images)

coveralls commented Aug 8, 2016 • edited Loading

jmartin-sul commented Aug 9, 2016

ggeisler commented Aug 9, 2016

ndushay Aug 9, 2016

Choose a reason for hiding this comment

jkeck Aug 9, 2016

Choose a reason for hiding this comment

ndushay commented Aug 9, 2016

snydman commented Aug 15, 2016

jkeck commented Aug 15, 2016

jvine commented Aug 15, 2016

LynnMcRae commented Aug 15, 2016

ggeisler commented Aug 16, 2016

aeschylus commented May 18, 2016 •

edited by peetucket

Loading

aeschylus commented May 19, 2016 •

edited

Loading

coveralls commented Aug 8, 2016 •

edited

Loading