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.

Sign up for GitHub

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

Jump to bottom

coala-header.svg: Fix coala.io text rendering #54

Merged
merged 1 commit into from
Oct 28, 2018
Merged

coala-header.svg: Fix coala.io text rendering #54

merged 1 commit into from
Oct 28, 2018

Conversation

sangamcse
Copy link
Member

@sangamcse sangamcse commented Oct 20, 2018 •
edited
Loading

This combines coala-color.svg and coala-text.svg using
<image> SVG element which includes images inside SVG
document. It takes image links generated from GitHub in the
href of <image> tag.

Closes #53

@sangamcse sangamcse force-pushed the coala-header branch 2 times, most recently from bfb8c67 to bc1b309 Compare October 20, 2018 14:07
ayan-b
ayan-b approved these changes Oct 20, 2018
View reviewed changes
Copy link
Member

@ayan-b ayan-b left a comment

Choose a reason for hiding this comment

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

LGTM.

jayvdb
jayvdb requested changes Oct 21, 2018
View reviewed changes
Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

The diff is unreadable.

The actual content changes cant be seen.

Split this into two commits, one which changes the style, and the other which changes the content.

@sangamcse
Copy link
Member Author

@jayvdb TBH, these changes in SVG XML format is done by Inkscape which I tried to reduce. My only concern was the logo text rendering which was a bit odd in the previous version but this change looks little satisfying to my eyes. 😄

@sangamcse sangamcse force-pushed the coala-header branch 3 times, most recently from 496fee1 to 54396bd Compare October 21, 2018 16:42
jayvdb
jayvdb requested changes Oct 21, 2018
View reviewed changes
artwork/logo/coala-header.svg Show resolved Hide resolved
artwork/logo/coala-header.svg Outdated
xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
viewBox="0 0 193.2 41.099998"
version="1.1"
id="svg3781"
Copy link
Member

Choose a reason for hiding this comment

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

the old id value was better

artwork/logo/coala-header.svg Outdated Show resolved Hide resolved
@jayvdb
Copy link
Member

jayvdb commented Oct 21, 2018

the commit message of ead8a4c83d6 is a bit odd.

"betterSVG."

ALso version="1.1" is moved twice in these two patches - returning to the same spot in the second patch.

Can we use https://developer.mozilla.org/en-US/docs/Web/SVG/Element/image to put 'coala.io' text in a separate SVG file which is re-usable , and also to re-use the real coala logo svg instead of creating a copy?

It would be better to try using https://developer.mozilla.org/en-US/docs/Web/SVG/Element/text for the 'coala.io'

@sangamcse
Copy link
Member Author

@jayvdb, I agree. Tried creating one at https://github.com/sangamcse/artwork/blob/coala-text/artwork/logo/coala_io_text.svg. Is it ok?

Can we use https://developer.mozilla.org/en-US/docs/Web/SVG/Element/image to put 'coala.io' text in a separate SVG file which is re-usable, and also to re-use the real coala logo SVG instead of creating a copy?

@jayvdb
Copy link
Member

jayvdb commented Oct 24, 2018

The o in .io isnt rendering for me in Falkon.

@ayan-b can you check?

@sangamcse sangamcse mentioned this pull request Oct 24, 2018
Closed
@sangamcse
Copy link
Member Author

Can we use https://developer.mozilla.org/en-US/docs/Web/SVG/Element/image to put 'coala.io' text in a separate SVG file which is re-usable , and also to re-use the real coala logo svg instead of creating a copy?

Created an issue for separate coala.io text. #56

@sangamcse sangamcse force-pushed the coala-header branch 6 times, most recently from 2ac8733 to e657579 Compare October 27, 2018 16:16
@sangamcse
coala-header.svg: Combine coala logo & text SVG
e657579 
This combines `coala-color.svg` and `coala-text.svg` using
`<image>` SVG element which includes images inside SVG
document. It takes image links generated from GitHub in the
`href` of `<image>` tag.

Closes coala#53
@jayvdb
Copy link
Member

jayvdb commented Oct 28, 2018

ack e657579

@jayvdb
Copy link
Member

jayvdb commented Oct 28, 2018

@gitmate-bot ff

@jayvdb
Copy link
Member

jayvdb commented Oct 28, 2018

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

@jayvdb jayvdb merged commit e657579 into coala:master Oct 28, 2018
@jayvdb
Copy link
Member

jayvdb commented Oct 28, 2018

Automated fastforward with GitMate.io was successful! 🎉

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

@jayvdb jayvdb jayvdb approved these changes

@ayan-b ayan-b ayan-b approved these changes

Assignees
No one assigned
Labels
size/S
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sangamcse @jayvdb @ayan-b