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.

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

Update globe and add hemispheres to be more inclusive #35

Closed
wants to merge 2 commits into from

Conversation

micahilbery
Copy link
Contributor

Add Eastern and Western hemisphere icons and make the default globe icon more central.

@xuv
Copy link
Member

xuv commented Feb 25, 2018

Hmmm. Although they work really great at bigger sizes, I find that it's hard to figure out what it is at smaller sizes. Except maybe for globe-w.

I really wonder if we should try to simplify them to the max. Removing as much detail as possible. And keep only 2 or 3 large shapes that indicate which side on the globe we are on.

Probably, the result won't be geographically correct, but that's not a problem. Just, when I look at these. I think Africa should be more detached from Asia and Europe to display well. Antartica, though lovely to have there, is maybe not giving much clue.

What do you think?

screen shot 2018-02-25 at 15 59 02

screen shot 2018-02-25 at 15 58 34

screen shot 2018-02-25 at 15 57 18

@tessus
Copy link
Member

tessus commented Feb 28, 2018

This PR changes files in the directory which I deleted. I hope there didn't go any other commits into the fork-awesome directory. Please note that this directory was always a dead end. (One of the reasons why I deleted it.)

Maybe we should have a guide book, where & how to add or change icons:

afaik the procedure is as follows:

  • scss and less files are changed/created in the directories scss and less
  • the new css files are created by the command line tool less
  • the new scss, less, and css files are commited

@tessus
Copy link
Member

tessus commented Feb 28, 2018

I just saw that 1426943 also changed files in the fork-awesome directory.

Was this change ever propagated to the real less/scss/css files? (I couldn't find anything in the logs.)

@xuv
Copy link
Member

xuv commented Feb 28, 2018

@tessus For sure in a later built. If not Medium and Medium-square would not show up in the doc. And they do: https://forkawesome.github.io/Fork-Awesome/icon/medium/

@micahilbery
Copy link
Contributor Author

micahilbery commented Mar 12, 2018

globes-lg
globes
Better?

@xuv
Copy link
Member

xuv commented Mar 12, 2018

Yes. Way better. Totally works at small sizes.

@micahilbery
Copy link
Contributor Author

micahilbery commented Mar 12, 2018

I am trying to update this but I have no idea how to fix this branch and get it so I can update it. I'm not familiar enough with git.

@tessus
Copy link
Member

tessus commented Mar 13, 2018

You created a branch globes and that branch should still be on your local machine.

You have to checkout that branch: git checkout globes

If you do not have it locally, pull all the latest changes or clone this repo. Then checkout the globes branch. However, please note that the directory fork-awesome does no longer exist in master, thus you have to make sure that the changes you made in the fork-awesome directory are done in the right places instead. Then you should delete the fork-awesome directory from the globes branch, otherwise it will be recreated in master during the merge.

If you need additional help with git, please let me know. You can also send me an email, if that's more convenient.

@tessus
Copy link
Member

tessus commented Mar 16, 2018

It looks like you haven't applied the changes that were originally in the fork-awesome directory to the correct files. After the second commit only a few files remain, but they conflict with the current ones in master.

Unfortunately I do not know how to add icons otherwise I'd help you to start applying the changes to current master.

@micahilbery
Copy link
Contributor Author

I think we should close this in favor of #67

@tessus
Copy link
Member

tessus commented Apr 11, 2018

I think we should close this in favor of #67

Yes, makes sense. Thank you for the new PR.

@tessus tessus closed this Apr 11, 2018
@tessus tessus deleted the globes branch April 13, 2018 03:39
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.

3 participants