Skip to content
This repository has been archived by the owner on Sep 2, 2020. It is now read-only.

Add 3 items to COPY_ATTRIBUTES which are used by iOS. #112

Merged
merged 2 commits into from
Dec 11, 2017
Merged

Add 3 items to COPY_ATTRIBUTES which are used by iOS. #112

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3114376
Add 3 items to COPY_ATTRIBUTES which are used by iOS.
montehurd Dec 8, 2017
7fa917b
Add tests for 'data-file-width', 'data-file-height' and 'data-image-g…
montehurd Dec 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/transform/LazyLoadTransform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ const IMAGE_LOADED_CLASS = 'pagelib_lazy_load_image_loaded' // Download complete

// Attributes copied from images to placeholders via data-* attributes for later restoration. The
// image's classes and dimensions are also set on the placeholder.
const COPY_ATTRIBUTES = ['class', 'style', 'src', 'srcset', 'width', 'height', 'alt']
// The 3 data-* items are used by iOS.
const COPY_ATTRIBUTES = ['class', 'style', 'src', 'srcset', 'width', 'height', 'alt',
'data-file-width', 'data-file-height', 'data-image-gallery'
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a pretty harmless change. It's entirely up to your discretion but you may want to update tests for these new attributes. We currently validate all others and don't have to worry about breaking things so much when a change is made.

Just an aside and not a request for revision: on line 77, these attributes will be copied to data-data-* which is a bit clumsy. On the other hand, they can be handled generically without any other changes which is great.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ya I had to account for the data-data prefix when implementing tap handling wikimedia/wikipedia-ios@8d50837 on the placeholders (ie if a user taps the placeholder span it was nice to have that still try to show the native gallery for that placeholder's image). Agreed it's a bit clumsy, but figured we could circle back later if we refactor in future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@niedzielski Tests added :)

]

// Small images, especially icons, are quickly downloaded and may appear in many places. Lazily
// loading these images degrades the experience with little gain. Always eagerly load these images.
Expand Down