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

Fix border loading issues #11

Merged
merged 1 commit into from Sep 12, 2019
Merged

Fix border loading issues #11

merged 1 commit into from Sep 12, 2019

Conversation

@EvanHahn
Copy link
Collaborator

EvanHahn commented Sep 12, 2019

tl;dr: before this change, the poster borders would sometimes disappear and reappear randomly—no longer! See #3 for more.

Three things I learned while working on this change that are relevant here:

  1. The paper size (like US Letter, or A4) is called aspectRatio internally.

  2. The poster borders are implemented as background images, one per aspect ratio.

  3. There are several "dormant" features, such as the ability to add a watermark to the result. They are dormant because their code paths are in the source code but unused.

Borders would sometimes fail to show up, which was because the background image sometimes failed to load properly. This change:

  1. Loads all 4 border images upfront. The border can still be missing while these images load (probably most noticeable on page load or on a slow connection), but the canvas will re-render as soon as any image loads.

  2. Removes the unused image upload and watermark features. I was changing a few things about how images worked and didn't want to do it twice. Doing this let me remove a bunch of things that were only used by these code paths.

  3. Each aspect ratio has a different border image; this change makes that relationship more explicit. (In the code, that means I moved the border image URL into aspectRatioOpts.) That also meant that the border image URL is computed from the aspect ratio instead of being a separate stored property that needs to be kept in sync.

Closes #3.

*tl;dr: before this change, the poster borders would sometimes disappear
and reappear randomly—no longer! See [issue #3][1] for more.*

Three things I learned while working on this change that are relevant
here:

1. The paper size (like US Letter, or A4) is called `aspectRatio`
   internally.

2. The poster borders are implemented as background images, one per
   aspect ratio.

3. There are several "dormant" features, such as the ability to add a
   watermark to the result. They are dormant because their code paths
   are in the source code but unused.

Borders would sometimes fail to show up, which was because the
background image sometimes failed to load properly. This change:

1. Loads all 4 border images upfront. The border can still be missing
   while these images load (probably most noticeable on page load or on
   a slow connection), but the canvas will re-render as soon as any
   image loads.

2. Removes the unused image upload and watermark features. I was
   changing a few things about how images worked and didn't want to do
   it twice. Doing this let me remove a bunch of things that were only
   used by these code paths.

3. Each aspect ratio has a different border image; this change makes
   that relationship more explicit. (In the code, that means I moved the
   border image URL into `aspectRatioOpts`.) That also meant that the
   border image URL is computed from the aspect ratio instead of being a
   separate stored property that needs to be kept in sync.

[1]: #3
@EvanHahn EvanHahn requested a review from dtoakley Sep 12, 2019
Copy link
Collaborator

dtoakley left a comment

Nice, love . it. Thanks @EvanHahn. LMTD

@dtoakley dtoakley merged commit e042382 into master Sep 12, 2019
5 checks passed
5 checks passed
Header rules - 350poster No header rules processed
Details
Pages changed - 350poster 8 new files uploaded
Details
Redirect rules - 350poster No redirect rules processed
Details
Mixed content - 350poster No mixed content detected
Details
netlify/350poster/deploy-preview Deploy preview ready!
Details
@dtoakley dtoakley deleted the fix_border_loading_issues branch Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.