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

Taiga #326

Merged
merged 16 commits into from
Oct 22, 2019
Merged

Taiga #326

merged 16 commits into from
Oct 22, 2019

Conversation

wuxmedia
Copy link
Contributor

taiga and tutanota added, possibly

@wuxmedia
new file: tutanova.svg
0c095d1
@wuxmedia
modified: README.md
e1cab96 
	new file:   images/reference/tutanota.svg
	renamed:    images/svg/tutanova.svg -> images/svg/tutanota.svg
@wuxmedia
modified: README.md
415fdf2 
	new file:   images/reference/taiga.svg
	new file:   images/svg/taiga.svg
@wuxmedia
modified: README.md
8e768b0
@wuxmedia
Copy link
Contributor Author

OMG no conflicts.

@alexanderadam
Copy link
Contributor

alexanderadam commented Oct 17, 2019 •
edited
Loading

If I may say that: you did some great contributions! 👍

tutanota reference & taiga reference

@wuxmedia
Copy link
Contributor Author

wuxmedia commented Oct 17, 2019 •
edited
Loading

could this be accepted? I've caught a bit of a minimal SVG bug.
as in I'd like to do more, but this is kind of a sticking point

@edent
Copy link
Owner

edent commented Oct 18, 2019

I'll take a look this weekend. Thank you for all your contributions 😁

@wuxmedia
Copy link
Contributor Author

sure - sorry I sounded a bit pushy! I was just making sure I hadn't made some sort of mistake :D

edent
edent requested changes Oct 21, 2019
View reviewed changes
Copy link
Owner

@edent edent left a comment

Choose a reason for hiding this comment

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

Few small changes, then good to go.

images/svg/tutanota.svg Outdated
@@ -0,0 +1,8 @@
<svg
xmlns="http://www.w3.org/2000/svg"
aria-label="..." role="img"
Copy link
Owner

Choose a reason for hiding this comment

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

This ARIA label needs to be completed.

images/svg/tutanota.svg Outdated
width="512" height="512"
rx="15%"
fill="#fff"/>
<path d="M113 97c-17 0-31 14.09-31.455 31.451v290.461c0 1.102.063 2.212.17 3.284a1700 1700 0 0037-14c91-36 165-67 165-102 0-1.12-.077-2.25-.236-3.38-4.717-34.512-87.191-45-87-61.028.01-.84.248-1.719.754-2 9-17 49-16 63-17 14-1 48-.996 50-11.336.048-.32.078-.637.078-.956.039-9-23-13-23-13s28 4 28 15c0 .54-.066 1.1-.217 1.67-3.055 11-28 14-44 14-15.615.782-39 2-39 10.186-.01.445.067.908.226 1 3 11 90 16 146 45 32 16 48 44 55 73V128c0-17-14-31-31-31H113z" fill="#a01e20"/></svg>
Copy link
Owner

Choose a reason for hiding this comment

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

Remove this newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which line? (number)

Copy link
Owner

Choose a reason for hiding this comment

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

At the end of line 7. fill="#fff"/>\n<path can become fill="#fff"/><path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok done.

images/svg/taiga.svg Outdated Show resolved Hide resolved
images/reference/taiga.svg Outdated Show resolved Hide resolved
images/reference/tutanota.svg Outdated Show resolved Hide resolved
@wuxmedia
Copy link
Contributor Author

do I make the readme up to the current then?

@wuxmedia
Update tutanota.svg
6e6e985 
killed newline
@wuxmedia
Copy link
Contributor Author

i don't know how to resolve this conflict.

@jmb
Copy link
Contributor

jmb commented Oct 22, 2019

It should show you the conflicting lines when you click into the conflict. It's probably just the fact that another icon has been added to the README in the same place as yours - you can probably accept both changes and put your icon on the line below. This might help explain: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/resolving-a-merge-conflict-on-github

@wuxmedia
Copy link
Contributor Author

I understand now. didn't know one could edit that file. I think I diverged from the master file though.
oh well

@wuxmedia wuxmedia requested a review from edent October 22, 2019 09:18
jmb
jmb reviewed Oct 22, 2019
View reviewed changes
README.md Outdated
@@ -177,6 +178,9 @@ Say thanks!
<td>Sentry<br><img src="https://edent.github.io/SuperTinyIcons/images/svg/sentry.svg" width="125" title="Sentry" /><br>507 Bytes</td>
<td>Behance<br><img src="images/svg/behance.svg" width="125" title="Behance" /><br>916 Bytes</td>
</tr>
<tr>
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably want to remove the </tr> and <tr> here as there is still space in the row above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok thanks! didn't see that :D

@jmb
Copy link
Contributor

jmb commented Oct 22, 2019

I think this looks ok (in terms of resolving the conflict). You might want to put the Taiga icon in the readme in the previous row rather than starting a new one though as there is space - see the comment I've put in the change!

@wuxmedia
Update README.md
47173d1 
rejigged tables
wuxmedia
wuxmedia commented Oct 22, 2019
View reviewed changes
Copy link
Contributor Author

@wuxmedia wuxmedia left a comment

Choose a reason for hiding this comment

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

pls check?

README.md Outdated
@@ -177,6 +178,9 @@ Say thanks!
<td>Sentry<br><img src="https://edent.github.io/SuperTinyIcons/images/svg/sentry.svg" width="125" title="Sentry" /><br>507 Bytes</td>
<td>Behance<br><img src="images/svg/behance.svg" width="125" title="Behance" /><br>916 Bytes</td>
</tr>
<tr>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok thanks! didn't see that :D

@wuxmedia
Copy link
Contributor Author

so how would I avoid having conflicts due updates made to readme.md (other commits coming it)? perhaps update my local repo to current ? Or just hack the file and merge?

@jmb
Copy link
Contributor

jmb commented Oct 22, 2019

Yes, you either have to update your repo from the master, or just use the merge tools. I don't know "the best" way - I'm just a hobbyist and tend to fumble my way around git!

@edent
Copy link
Owner

edent commented Oct 22, 2019

@wuxmedia You currently have tutanota.svg saying aria-label="..."

You need to change that to aria-label="Tutanota" and then update the filesize in the README.

See https://raw.githubusercontent.com/edent/SuperTinyIcons/47173d1a4b8260cc07184bd4577bacae86f328b5/images/svg/tutanota.svg

I'll merge once that is done. Cheers :-)

@wuxmedia
Update tutanota.svg
69a1228 
added Aria label
@wuxmedia
Update README.md
8a35744 
updated tutanota bytes
@wuxmedia
Copy link
Contributor Author

ok Done!

@edent edent merged commit d86da19 into edent:master Oct 22, 2019
@edent
Copy link
Owner

edent commented Oct 22, 2019

All done. In future, probably best to add only one icon per PR.

@wuxmedia wuxmedia deleted the taiga branch October 22, 2019 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmb jmb jmb left review comments

@edent edent Awaiting requested review from edent

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wuxmedia @alexanderadam @edent @jmb