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 image converter #735

Merged
merged 6 commits into from
Jun 2, 2020
Merged

add image converter #735

merged 6 commits into from
Jun 2, 2020

Conversation

yulia-dnistrian
Copy link
Contributor

@yulia-dnistrian yulia-dnistrian commented May 28, 2020

Description:

Example how to run the script

bash ./tools/ConvertFiles.sh

Before running the script you need to install "imagemagick", "jxrlib", "jq"

  • brew install imagemagick
  • brew install jxrlib
  • brew install jq

Linting:

  • No linting errors

Tests:

  • E2E tests (npm test run with e2e)
  • Manual tests
  • Accessibility tests (no new react-axe errors in console)

Documentation:

  • Requires documentation updates
  • Requires Storybook component updates

@aChanEP
Copy link
Contributor

aChanEP commented May 28, 2020

@shaunmaharaj: Before merging we should update docs and have some information surrounding this new util file

Copy link
Contributor

@aChanEP aChanEP left a comment

Choose a reason for hiding this comment

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

Screen Shot 2020-05-28 at 12 19 51 PM

May want to include some clean up logic here for when the tool is run twice

tools/ConvertFiles.sh Outdated Show resolved Hide resolved
tools/ConvertFiles.sh Outdated Show resolved Hide resolved
tools/ConvertFiles.sh Outdated Show resolved Hide resolved
src/ep.config.json Outdated Show resolved Hide resolved
tools/ConvertFiles.sh Outdated Show resolved Hide resolved
@shaunmaharaj
Copy link
Contributor

Thanks for working on this @yulia-dnistrian. I'll add documentation for running the script to our docs once we're ready. Thank you for posting the tutorial in the PR notes.

@aChanEP
Copy link
Contributor

aChanEP commented Jun 1, 2020

Screen Shot 2020-06-01 at 1 27 07 PM

recving this error here with the jpegxr command.

EDIT: Need to install jxrlib

@aChanEP
Copy link
Contributor

aChanEP commented Jun 1, 2020

Screen Shot 2020-06-01 at 3 26 40 PM

Can we get rid of the output from the jpgxr lib?
Maybe just place an output that indicates to the user that things were successful or not if that is the case.

@aChanEP
Copy link
Contributor

aChanEP commented Jun 1, 2020

Screen Shot 2020-06-01 at 3 31 20 PM

Can we change Placeholders to PlaceholdersPng for consitency or just have on placeholders file with all the images we converted.

Copy link
Contributor

@aChanEP aChanEP left a comment

Choose a reason for hiding this comment

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

LGTM!

@aChanEP
Copy link
Contributor

aChanEP commented Jun 2, 2020

Thanks for working on this @yulia-dnistrian. I'll add documentation for running the script to our docs once we're ready. Thank you for posting the tutorial in the PR notes.

We should update Bellevie with JPEGXR format (missing when conversion was done manually) and VESTRI with all the next-gen formats. It should be simple now :). I will go ahead and do that once this is merged.

@shaunmaharaj shaunmaharaj merged commit fca84ea into master Jun 2, 2020
@shaunmaharaj shaunmaharaj deleted the RS-834-image-converter branch June 2, 2020 14:40
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