mirador-annotations icon indicating copy to clipboard operation
mirador-annotations copied to clipboard

Add image annotation insert/edit to mirador-annotations

Open MImranAsghar opened this issue 3 years ago • 3 comments

  • This image insert/edit input dialog is needed to extend the annotation support to adding image annotation.
  • These image annotation insert/edit changes attempt to implement image insert/edit workflow similar to Mirador 2 for the image annotation. For example, once a url is entered then clicking away from the url field will update the width and height inputs with the image width and height that the url corresponds to.
  • The other part of image annotations would be the image annotation card (list of image annotations displayed on a card on hovered annotations) which I am currently working on that can be added as a separate plugin (to be used with this one) or to mirador 3 eventually

MImranAsghar avatar Apr 06 '21 15:04 MImranAsghar

@MImranAsghar Thanks for working on this. I have some comments that are strictly about the visual design; I will leave it to others to weigh in on the more technical aspects (though I might have thoughts on the overall user experience as you get additional parts of the feature completed).

I realize that you are probably more focused on the functionality of this feature than the visual design at this point (understandably) but I want to make sure contributors are conscious of following existing Mirador 3 patterns when creating new UI elements. Ideally we'd have some form of detailed guide for Mirador 3 visual design, which would make it easier for developers. We already have divergence in some of the other Mirador 3 plugins so lacking a detailed style guide I want to point out these things so we can minimize further divergence.

Screen Shot 2021-04-06 at 10 49 46 AM
  • A: This doesn't look like the correct header for a modal. See the Workspace options (... menu in Workspace sidebar) and look at either the Import workspace or Export workspace modals for an example of the model I think we should follow. The heading should be larger and use a different font weight variant, I believe.
  • A: Also, we are using sentence case for UI labels in Mirador 3, so this should be "Insert image" (lowercase "image" and I think we could drop the "Edit" part since if the user edits an existing image annotation, they will presumably see the fields filled out and it'll obvious they are editing).
  • B: Since these are headings, not field labels, I believe they should be larger and darker than the placeholder text in the input field. I'm not sure we have a good example of this pattern yet, but I'd suggest using a heading similar to what you are currently using for the "Insert/Edit Image" header for these headings.
  • B: Not a big deal, but I think we could add the word "Image" to both of these labels for a bit more clarity: "Image source" and "Image dimensions"
  • C: "Url" should be in all caps: "URL"
  • D: The checkbox and label of Constrain proportions should have more horizontal space between it and the Height input. Currently the checkbox is closer to the Height input than the Width and Height inputs are to each other. The inputs are associated with each other so visually they should appear as a group, and the Constrain proportions checkbox and label should visually appear as a separate (though related) control, which means there needs to be at least ~20 or 30px between the Height input and the checkbox.
  • E: "Proportions" should be lowercase "proportions"
  • F: See the Import workspace or Export workspace modals for correct styling of these buttons. "Add" should be represented as the primary action, and the colors and hover effects of the buttons should be different. Hopefully you can fairly easily just use the styles applied to the Import workspace or Export workspace modal buttons and get the correct styling.

ggeisler avatar Apr 06 '21 18:04 ggeisler

@MImranAsghar Thanks for working on this. I have some comments that are strictly about the visual design; I will leave it to others to weigh in on the more technical aspects (though I might have thoughts on the overall user experience as you get additional parts of the feature completed).

I realize that you are probably more focused on the functionality of this feature than the visual design at this point (understandably) but I want to make sure contributors are conscious of following existing Mirador 3 patterns when creating new UI elements. Ideally we'd have some form of detailed guide for Mirador 3 visual design, which would make it easier for developers. We already have divergence in some of the other Mirador 3 plugins so lacking a detailed style guide I want to point out these things so we can minimize further divergence.

Screen Shot 2021-04-06 at 10 49 46 AM
  • A: This doesn't look like the correct header for a modal. See the Workspace options (... menu in Workspace sidebar) and look at either the Import workspace or Export workspace modals for an example of the model I think we should follow. The heading should be larger and use a different font weight variant, I believe.
  • A: Also, we are using sentence case for UI labels in Mirador 3, so this should be "Insert image" (lowercase "image" and I think we could drop the "Edit" part since if the user edits an existing image annotation, they will presumably see the fields filled out and it'll obvious they are editing).
  • B: Since these are headings, not field labels, I believe they should be larger and darker than the placeholder text in the input field. I'm not sure we have a good example of this pattern yet, but I'd suggest using MuiTypography-h3 for these headings.
  • B: Not a big deal, but I think we could add the word "Image" to both of these labels for a bit more clarity: "Image source" and "Image dimensions"
  • C: "Url" should be in all caps: "URL"
  • D: The checkbox and label of Constrain proportions should have more horizontal space between it and the Height input. Currently the checkbox is closer to the Height input than the Width and Height inputs are to each other. The inputs are associated with each other so visually they should appear as a group, and the Constrain proportions checkbox and label should visually appear as a separate (though related) control, which means there needs to be at least ~20 or 30px between the Height input and the checkbox.
  • E: "Proportions" should be lowercase "proportions"
  • F: See the Import workspace or Export workspace modals for correct styling of these buttons. "Add" should be represented as the primary action, and the colors and hover effects of the buttons should be different. Hopefully you can fairly easily just use the styles applied to the Import workspace or Export workspace modal buttons and get the correct styling.

ggeisler avatar Apr 06 '21 18:04 ggeisler

Hi, Thanks for pointing these out. All your comments look very useful, I will start looking into these one by one and make the required changes 👍

MImranAsghar avatar Apr 06 '21 18:04 MImranAsghar