Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

fix close button #2143

Merged
merged 1 commit into from
Mar 14, 2014
Merged

fix close button #2143

merged 1 commit into from
Mar 14, 2014

Conversation

chadwhitacre
Copy link
Contributor

The close button for accounts elsewhere was not implemented properly in #2142. Due to the way our CDN is implemented, CSS background URLs need to be specificed relative to the assets/%version directory to work properly in both development and production.

I also switched from _ to - to match existing naming conventions.

The close button for accounts elsewhere was not implemented properly in
 #2142. Due to the way our CDN is implemented, CSS background URLs need
to be specificed relative to the /assets/%version directory to work
properly in both development and production.

I also switched from _ to - to match existing naming conventions.
@chadwhitacre
Copy link
Contributor Author

Ping @rummik @seanlinsley @clone1018 @ESWAT.

@seanlinsley
Copy link
Contributor

In the same file we have a url() using /assets/. So... which is right?

@chadwhitacre
Copy link
Contributor Author

@seanlinsley Good catch. That payments-by class is actually unused at the moment, so I didn't bother updating it. I'm comfortable removing payments-by for now (it came from #2053, which this PR is derived from; we can add it back when needed), but it turns out that there's one other place where we use /assets/. Let me look into that other case ...

@chadwhitacre chadwhitacre mentioned this pull request Mar 14, 2014
@chadwhitacre
Copy link
Contributor Author

@seanlinsley payments-by dealt with in #2144.

@chadwhitacre
Copy link
Contributor Author

I've confirmed that the .on-confirm .participant case is indeed broken in production. Since the heart is the old heart anyway I'm going to also update the heart we're using. See also #712.

screen shot 2014-03-14 at 3 49 59 pm

screen shot 2014-03-14 at 3 50 25 pm

@chadwhitacre chadwhitacre mentioned this pull request Mar 14, 2014
@chadwhitacre
Copy link
Contributor Author

Okay @seanlinsley, third and final /assets/ reference fixed in #2145. This chain is ready to go! :-)

seanlinsley added a commit that referenced this pull request Mar 14, 2014
@seanlinsley seanlinsley merged commit c1d50ea into master Mar 14, 2014
@seanlinsley seanlinsley deleted the fix-close-button branch March 14, 2014 20:13
@ESWAT
Copy link

ESWAT commented Mar 14, 2014

Hmm OK. I grabbed that image from coinbase branch, so might have to keep this in mind once that's merged (there was another image or two in there).

@chadwhitacre
Copy link
Contributor Author

@ESWAT Yeah, I think we're good with #2144 and #2145. :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants