Skip to content
This repository has been archived by the owner on Dec 27, 2023. It is now read-only.

Addressed issue #87 #94

Merged
merged 9 commits into from
Sep 13, 2014
Merged

Addressed issue #87 #94

merged 9 commits into from
Sep 13, 2014

Conversation

vladfulgeanu
Copy link
Contributor

@@ -416,7 +416,6 @@ var privlyNetworkService = {
*/
initializeNavigation: function() {
var domain = privlyNetworkService.contentServerDomain();
$(".home_domain").attr("href", domain);
Copy link
Member

Choose a reason for hiding this comment

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

grep for the home_domain class and update it as appropriate. You can changing the way it was being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there something else I should change?

@smcgregor
Copy link
Member

I don't think you are getting the expectation quite right on changing the banner. Perhaps it would be better to add a page to the "Help" application that explains what the user's content server is and what scripting environment they are currently viewing. This approach could allow us to build a trusted knowledge base inside the extension.

@smcgregor
Copy link
Member

You could then link the banner to the help page, which would further have details on how to view the pages defined by the content server instead of the extension.

@vladfulgeanu vladfulgeanu changed the title Addresed issue #87 Addressed issue #87 Sep 12, 2014
@smcgregor
Copy link
Member

Is there a reason you are using the same javascript as the Help/new.html page? I would probably use a different script. Maybe named content_server.js

@vladfulgeanu
Copy link
Contributor Author

The only reason is not doubling most of the code, those three lines are the only addition to new.js.

@@ -50,7 +50,7 @@
<span class="icon-bar">
</span>
</button>
<a class="navbar-brand lobster home_domain" href="" target="_blank">
<a class="navbar-brand lobster home_domain" href="../Help/content_server.html" target="_blank" title="This is your current content server">
Copy link
Member

Choose a reason for hiding this comment

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

This no longer needs the home_domain class and the target should no longer be specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? The banner should still indicate the current content server, which is done by targeting $(.home_domain), and it should also open in a new tab.

Copy link
Member

Choose a reason for hiding this comment

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

ah, ok. I thought you removed the home_domain logic.

I still think you should open in the same tab though since that is how people expect it to behave and the app is opening in the same context.

@smcgregor
Copy link
Member

OK, then rename the script to something that indicates it could be used in more than one page. When it is named the same thing as an HTML file, it is implicit that the HTML file owns the JS so you don't need to worry about breaking dependencies. When you change the file name to something not clearly related to a specific file, it won't cause confusion later.

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

2 participants