Skip to content

Add support for thumbnails using <thumbnail>#881

Merged
Chng-Zhi-Xuan merged 3 commits into
MarkBind:masterfrom
alyip98:876-thumb-vue
Jun 4, 2019
Merged

Add support for thumbnails using <thumbnail>#881
Chng-Zhi-Xuan merged 3 commits into
MarkBind:masterfrom
alyip98:876-thumb-vue

Conversation

@alyip98

@alyip98 alyip98 commented May 27, 2019

Copy link
Copy Markdown
Contributor

What is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [x] New feature

Resolves #876

Related PR on vue-strap:
MarkBind/vue-strap/pull#107

What is the rationale for this request?

What changes did you make? (Give an overview)
Added a new Vue component that allows authors to create thumbnails using images easily. Tests and user documentation included.

Is there anything you'd like reviewers to focus on?

Testing instructions:
The components section in the user guide contains a demo of the feature.

Proposed commit message: (wrap lines at 72 characters)
To use an image as a thumbnail, we have to crop and rescale the image
by wrapping it in a div and using CSS. This can be especially
cumbersome with multiple images with different dimensions.

Let's add a built in feature that supports convenient creation of
thumbnails using a tag. It only accepts one dimension attribute
size, rather than a separate width and height, and crops the
supplied image into a square or optionally a circle with minimal
hassle from the author.

Let's also extend that feature to also allow the author to supply text,
icons or emojis in place of an image.

@alyip98

alyip98 commented May 27, 2019

Copy link
Copy Markdown
Contributor Author

Just realized there's a conflict, I'll wait for the PR on vue-strap first

@Chng-Zhi-Xuan Chng-Zhi-Xuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most of the review comments is on vue-strap repo, however there are corrections that need to be made here as well:

  • Commit titles should not be prefixed with <thumb>.
  • Commit titles should be more descriptive (eg. Add thumbnail in "Using Components")
  • Author a proposed commit message, summarising the intent / use case for this new feature.
  • Use the full component name in commit messages and titles
  • Your feature does not warrant a separate test site folder. Authoring a stand-alone test-thumbnail.md within test_site is sufficient.
  • Since the PR on the vue-strap repo is [WIP], this should be as well.

Comment thread docs/userGuide/syntax/thumbnails.mbdf Outdated
@alyip98 alyip98 changed the title Add support for thumbnails using <thumb> [WIP] Add support for thumbnails using <thumb> May 29, 2019
@alyip98 alyip98 force-pushed the 876-thumb-vue branch 2 times, most recently from 56e82f9 to d96d627 Compare May 29, 2019 09:35
@alyip98 alyip98 changed the title [WIP] Add support for thumbnails using <thumb> [WIP] Add support for thumbnails using <thumbmail> May 29, 2019
@alyip98 alyip98 changed the title [WIP] Add support for thumbnails using <thumbmail> [WIP] Add support for thumbnails using <thumbnail> May 29, 2019
@alyip98 alyip98 changed the title [WIP] Add support for thumbnails using <thumbnail> [RFC] Add support for thumbnails using <thumbnail> May 29, 2019
@alyip98 alyip98 requested a review from Chng-Zhi-Xuan May 29, 2019 12:33
Comment thread docs/userGuide/syntax/thumbnails.mbdf Outdated
Comment thread docs/userGuide/syntax/thumbnails.mbdf
Comment thread docs/userGuide/syntax/thumbnails.mbdf Outdated
Comment thread src/lib/markbind/src/parser.js Outdated
Comment thread test/functional/test_site/site.json Outdated
Comment thread src/lib/markbind/src/parser.js Outdated
@Chng-Zhi-Xuan

Copy link
Copy Markdown
Contributor

Have to update + review this PR before approval for MarkBind/vue-strap#107 can be given.

@alyip98 alyip98 force-pushed the 876-thumb-vue branch 5 times, most recently from 5241497 to 9b35644 Compare June 3, 2019 06:50
Comment thread test/functional/test_site/testThumbnails.md Outdated
Comment thread docs/userGuide/syntax/thumbnails.mbdf Outdated
Comment thread docs/userGuide/syntax/thumbnails.mbdf Outdated
@alyip98 alyip98 force-pushed the 876-thumb-vue branch 3 times, most recently from c6b06f8 to 15242a3 Compare June 3, 2019 07:20
@damithc

damithc commented Jun 3, 2019

Copy link
Copy Markdown
Contributor

image
Add a blank line above this line to parse it as Markdown

image
Perhaps show the same icons without circle as another row?

Comment thread test/functional/test_site/testThumbnails.md Outdated
Comment thread test/functional/test_site/testThumbnails.md
Comment thread docs/userGuide/syntax/thumbnails.mbdf Outdated
To use an image as a thumbnail, we have to crop and rescale the image
by wrapping it in a div and using CSS. This can be especially
cumbersome with multiple images with different dimensions.

Let's add a built in feature that supports convenient creation of
thumbnails using a <thumb> tag. It only accepts one dimension attribute
`size`, rather than a separate `width` and `height`, and crops the
supplied image into a square or optionally a circle with minimal
hassle from the author.

Let's also extend that feature to also allow the author to supply text,
icons or emojis in place of an image.
@alyip98 alyip98 force-pushed the 876-thumb-vue branch 3 times, most recently from 8c090a2 to 3e24b99 Compare June 4, 2019 03:53
<thumbnail text="blue" border='1px solid black' font-color="#669" circle />
<thumbnail text="maroon" border='1px solid black' font-color="maroon" font-size="25" circle />

{{test}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is this {{test}} variable used for?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I probably added it very early on when testing something else and forgot about it, will remove

@Chng-Zhi-Xuan Chng-Zhi-Xuan changed the title [RFC] Add support for thumbnails using <thumbnail> Add support for thumbnails using <thumbnail> Jun 4, 2019
@Chng-Zhi-Xuan Chng-Zhi-Xuan merged commit 6fb0767 into MarkBind:master Jun 4, 2019
@Chng-Zhi-Xuan Chng-Zhi-Xuan added this to the v2.5.1 milestone Jun 4, 2019
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.

Add new Vue component to implement circle cutout functionality

3 participants