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

Replace "elephant_ui_plane.svg" asset with Custom mascot #8766

Merged
merged 1 commit into from Oct 7, 2018

Conversation

@Shleeble
Copy link
Contributor

commented Sep 24, 2018

This is my first pass of this feature patch... I'm new to Ruby.. I'll test and patch bugs tonight.

@ykzts
ykzts approved these changes Sep 25, 2018
@Shleeble

This comment has been minimized.

Copy link
Contributor Author

commented Sep 26, 2018

There is a hardcoded path for the file in

/Users/shlee/Documents/GitHub/mastodon/app/javascript/styles/mastodon/modal.scss
18,32: background: url('../images/elephant_ui_plane.svg') no-repeat left bottom / contain;

and

/Users/shlee/Documents/GitHub/mastodon/app/javascript/mastodon/features/compose/index.js
15,46: import elephantUIPlane from '../../../images/elephant_ui_plane.svg';

but I don't understand how to make them dynamic yet.

@MaciekBaron

This comment has been minimized.

Copy link
Collaborator

commented Sep 27, 2018

Maybe I'm a bit nitpicky but could we squash these commits into one?

@Shleeble

This comment has been minimized.

Copy link
Contributor Author

commented Sep 27, 2018

Maybe I'm a bit nitpicky but could we squash these commits into one?

I need to get my head around git - I'd assume this would be squashed at merge? or can I squash them before hand?

Update: steps below worked as expected. I'll aim to keep future PRs in similar condition.

@MaciekBaron

This comment has been minimized.

Copy link
Collaborator

commented Sep 27, 2018

The merge would bring all your commits into master.

The easiest way would probably be running:

git reset --soft HEAD~7
git commit -m "Replace SVG asset with Custom mascot"

And then doing a force push to your branch (with -f). This why we have one, nicely labelled commit that explains your change in the log.

@MaciekBaron MaciekBaron self-requested a review Sep 27, 2018
@Gargron

This comment has been minimized.

Copy link
Member

commented Sep 27, 2018

One thing that comes to mind is, if you're planning to make all mascots customizable, naming the setting "mascot" is not going to age well. On the other hand... There are really a lot of elephants in the UI. If only some can be configured, it's going to be an inconsistency, but if you're going to customize all of them, that's a big mess too. I'm not sure if that can of worms should be opened at all?

@Shleeble

This comment has been minimized.

Copy link
Contributor Author

commented Sep 28, 2018

@Gargron mascot was used because that's the common reference in the code - but I understand it could be confusing.. Maybe it's better to talk about goals - I want to change this dude every now and then. as mentioned in #8525

image

Is your recommendation to edit the hardcoded paths/css directly and rebuild? this means I'm git stashing a few extra files or making a few extra modifications to a few extra files every upgrade.

The cleaner option might be expanding the official documentation with the recommend practices to modify there things.

@Shleeble

This comment has been minimized.

Copy link
Contributor Author

commented Sep 28, 2018

but if you're going to customize all of them, that's a big mess too. I'm not sure if that can of worms should be opened at all?

If you DO want to go down that path - maybe we need to move theming into it's own menu in administration.

right now... I'd be interested in quick/easy replacement for

Image based theming

  • favicon
  • Large Logo (TOP)
  • Small Logo (Middle)
  • Hero image (already tweakable)
  • Mascot dude (Corner)
  • Instance thumbnail (unseen)

but mascot dude is the only one I truly care about

image

@Gargron Gargron merged commit 2dba313 into tootsuite:master Oct 7, 2018
11 checks passed
11 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: check-i18n Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.3 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.3 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-webui Your tests passed on CircleCI!
Details
codeclimate All good!
Details
@nightpool

This comment has been minimized.

Copy link
Collaborator

commented Oct 7, 2018

@MaciekBaron @ashleyhull-versent we always squash PRs when merging them, so it's not very important to do so ahead of time.

Gargron added a commit that referenced this pull request Oct 12, 2018
Gargron added a commit that referenced this pull request Oct 12, 2018
Follow-up to #8766
@MayMeow

This comment has been minimized.

Copy link

commented Nov 8, 2018

is there any way to revert customized mascot back to default one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.