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

First version of "import" page is ready #27

Merged
merged 15 commits into from
Feb 21, 2023
Merged

First version of "import" page is ready #27

merged 15 commits into from
Feb 21, 2023

Conversation

alix-tz
Copy link
Owner

@alix-tz alix-tz commented Feb 1, 2023

Some "todo" remaining: left for later in #24 #25 and #26

Fixes #17

alix-tz and others added 4 commits February 1, 2023 16:47
see 'todo:' for the remaining elements to correct
* adding edit_uri settings

* fix edit_uri

* fix edit_uri
and config a new keymap and a css
for a nicer display
@HugoSchtr HugoSchtr self-assigned this Feb 3, 2023
@alix-tz alix-tz requested review from FloChiff and HugoSchtr February 8, 2023 12:33
Copy link
Collaborator

@HugoSchtr HugoSchtr left a comment

Choose a reason for hiding this comment

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

It looks very good, good job! I've only made a few comments on phrasing and references!

@HugoSchtr
Copy link
Collaborator

Well, I have requested changes, but I didn't know that it would block merging. 😅 Of course, no changes are required, my comments were only suggestions. 👍

using Hugo's method to create gifs
Copy link
Collaborator

@FloChiff FloChiff left a comment

Choose a reason for hiding this comment

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

On top of the comments by Hugo that I also mostly agree to (I am not sure about the place of the mapping part), here are the typos I saw in the document :

l. 42 --> I think the correct term is "manifest" and not "manifesto" ; I also think that "metadata" is plural and not singular so it should be "are visible" instead of "is visible"
l. 51 --> I think it should be a present "can" and not a conditional "could" as you also already used "can" right before
l. 79 --> it should be "Known" and not "Knwon"
l. 83 --> "useful" only has one 'l' at the end and "targeting" is written with only one 't' at the end

Also, around the lines 25-30 about the import of images, I have a recommendation for the tip related to a problem I sometimes encountered but you don't have to add it if you don't find it relevant --> when I upload big load of images (like 100 at the same time), it can make eScriptorium bug sometimes and it will not be able to upload one or two images in the mix, even though it load the images before and after. This is not a big issue as you can upload them afterwards but when you care about image orders, this is frustrating. So the advice regarding that would be to upload by little chunk (25 by 25 or something like that).

@alix-tz
Copy link
Owner Author

alix-tz commented Feb 9, 2023

Thank you both for your feedback! They are all very good remarks, I'll make the changes

* first draft for annotations export documentation

* adding gifs for illustrating the export section

* fixing filepaths

* Fixing typos in markdown syntax

* exporting model section added

* adding model exporting illustration

* fixing document title

* correcting typos or rephrasing
linting spaces
adding key display thanks to the keys extension ;)

* clarifying annotation vs. transcription (in partular now that there is a whole 'annotation' feature

* rephrase include/exclude region types

* adding titles to images

* spotted a typo in my correction #oups

* spotted a typo in my correction #oups

* spotted a typo in my correction #oups

* typos

* Typos and harmonizing markdown syntax

* titles for images and gif for downloading export

* adding gif export download

* deleting useless screenshot

* Fixing typos

* Typo

* adding Floriane as co-author

* update date

---------

Co-authored-by: Alix Chagué <[email protected]>
@alix-tz
Copy link
Owner Author

alix-tz commented Feb 13, 2023

Also, around the lines 25-30 about the import of images, I have a recommendation for the tip related to a problem I sometimes encountered but you don't have to add it if you don't find it relevant --> when I upload big load of images (like 100 at the same time), it can make eScriptorium bug sometimes and it will not be able to upload one or two images in the mix, even though it load the images before and after. This is not a big issue as you can upload them afterwards but when you care about image orders, this is frustrating. So the advice regarding that would be to upload by little chunk (25 by 25 or something like that).

@FloChiff : you know that you can reorder images by dragging the "image" card in /images?

@FloChiff
Copy link
Collaborator

FloChiff commented Feb 13, 2023

@alix-tz Well I do now !!! I discover a new thing everyday about eSc thanks to this tutorial 😄 !!!

@alix-tz
Copy link
Owner Author

alix-tz commented Feb 13, 2023

I had to rebase because the config in mkdocs.yml was not up-to-date (the rendering of admonitions was not good).

@alix-tz
Copy link
Owner Author

alix-tz commented Feb 13, 2023

Well, I have requested changes, but I didn't know that it would block merging. 😅 Of course, no changes are required, my comments were only suggestions. 👍

@HugoSchtr I don't understand why it's still requesting the validation of the reviews...

@alix-tz alix-tz requested a review from HugoSchtr February 13, 2023 11:10
Copy link
Collaborator

@HugoSchtr HugoSchtr left a comment

Choose a reason for hiding this comment

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

😎

@alix-tz
Copy link
Owner Author

alix-tz commented Feb 21, 2023

should we merge?

@alix-tz alix-tz merged commit bee2ee2 into main Feb 21, 2023
@alix-tz alix-tz deleted the fix/#17 branch February 21, 2023 16:51
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.

Walkthrough - Import
3 participants