Skip to content
This repository was archived by the owner on Jan 25, 2021. It is now read-only.

[blog sample data] New attempt with different images.#248

Merged
richard67 merged 5 commits intodevelopmentfrom
sample-data-more-images
Nov 22, 2020
Merged

[blog sample data] New attempt with different images.#248
richard67 merged 5 commits intodevelopmentfrom
sample-data-more-images

Conversation

@chmst
Copy link
Collaborator

@chmst chmst commented Nov 18, 2020

Pull Request for Issue #4 .

Summary of Changes

  • Re-Added lost lead typograpy in the language file.
  • Removed duplicate comment
  • Remove superfluous image params.
  • Add the new param for _alt_empty to most of the images - @brianteeman
  • Add alt text to two images

Add different images from the nasa imagery (including a backlink). Images are cut and size is optimised.

Testing Instructions

Install the blog sample data on a fresh joomla.

Expected result

kind of that:

grafik

Actual result

Only one image (the banner is used everywhere)

Documentation Changes Required

screens maybe

@chmst chmst requested review from bembelimen and hans2103 November 18, 2020 22:09
@chmst chmst changed the title New attempt with different images. [blog sample data] New attempt with different images. Nov 18, 2020
@richard67
Copy link
Member

@chmst You set the new "alt empty" flag to 1 in sample data for the articles' intro and full text images. Is that right?
@brianteeman What do you think?

@brianteeman
Copy link
Contributor

They are decorative so its ok but it would be nice to have an example in the sample data set that uses an alt description.

This is a useful guide to when an image is decorative etc https://www.w3.org/WAI/tutorials/images/decision-tree/

@chmst
Copy link
Collaborator Author

chmst commented Nov 20, 2020

Text data are always work for translators. But I have added two very short sample alt texts.

Copy link
Collaborator

@bembelimen bembelimen left a comment

Choose a reason for hiding this comment

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

If we're allowed to use the images license-wise, then it's good to go.

@richard67
Copy link
Member

@drmenzelit @bembelimen Your approval was based on code review (which is fine and sufficient for an approval)? Or did you also do a real test?

@drmenzelit
Copy link
Collaborator

Only code review this time

@hans2103
Copy link
Collaborator

code review

Copy link
Member

@richard67 richard67 left a comment

Choose a reason for hiding this comment

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

Last changes fix the issues I've found.

@richard67
Copy link
Member

Funny that 3 previous reviewers haven't found it.

@richard67
Copy link
Member

I have tested this PR ✅ with success.

@richard67 richard67 merged commit bbf7af8 into development Nov 22, 2020
@richard67 richard67 deleted the sample-data-more-images branch November 22, 2020 15:29
@richard67
Copy link
Member

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants