Issue 2383 - Added links to options page sidebar

Made the changes as outlined in #2383 and also fixed the broken version number. Note that the "releases" link doesn't work yet due to #2734.

[saroyanm, 2015-06-29 15:59:17]

https://codereview.adblockplus.org/29321206/diff/29321207/options.html
File options.html (right):

https://codereview.adblockplus.org/29321206/diff/29321207/options.html#newcode40
options.html:40: <a class="i18n_options_tab_general"></a>
Can you please describe what is the reason of using <a> tags instead of  <span>
here ?
And how are this changes connected with current issue ?
Also I do think we will need to use ::after Pseudo element for the tab icons in
future.

https://codereview.adblockplus.org/29321206/diff/29321207/options.js
File options.js (right):

https://codereview.adblockplus.org/29321206/diff/29321207/options.js#newcode636
options.js:636: ext.backgroundPage.sendMessage({
nit: please place the opening curly bracket in the new line for consistency.

https://codereview.adblockplus.org/29321206/diff/29321207/skin/options.css
File skin/options.css (right):

https://codereview.adblockplus.org/29321206/diff/29321207/skin/options.css#ne...
skin/options.css:243: display: block;
I guess this should be "display: flex;", consider "rtl".

[Thomas Greiner, 2015-06-29 16:34:12]

https://codereview.adblockplus.org/29321206/diff/29321207/options.html
File options.html (right):

https://codereview.adblockplus.org/29321206/diff/29321207/options.html#newcode40
options.html:40: <a class="i18n_options_tab_general"></a>
On 2015/06/29 15:59:17, saroyanm wrote:
> Can you please describe what is the reason of using <a> tags instead of 
<span>
> here ?
> And how are this changes connected with current issue ?

Semantically, it's a link so we should use the appropriate HTML element for
that. Furthermore this is a bit of preparation for #2409 to make those items
focusable.

> Also I do think we will need to use ::after Pseudo element for the tab icons
in
> future.

Agree, but that's out of scope for this ticket.

https://codereview.adblockplus.org/29321206/diff/29321207/options.js
File options.js (right):

https://codereview.adblockplus.org/29321206/diff/29321207/options.js#newcode636
options.js:636: ext.backgroundPage.sendMessage({
On 2015/06/29 15:59:17, saroyanm wrote:
> nit: please place the opening curly bracket in the new line for consistency.

I changed it but seeing that it's done differently in firstRun.js, I'd actually
recommend switching to the syntax from there to at least ensure consistency
within the adblockplusui repository.

https://codereview.adblockplus.org/29321206/diff/29321207/skin/options.css
File skin/options.css (right):

https://codereview.adblockplus.org/29321206/diff/29321207/skin/options.css#ne...
skin/options.css:243: display: block;
On 2015/06/29 15:59:17, saroyanm wrote:
> I guess this should be "display: flex;", consider "rtl".

Done.

[saroyanm, 2015-06-29 18:40:41]

LGTM

https://codereview.adblockplus.org/29321206/diff/29321207/options.html
File options.html (right):

https://codereview.adblockplus.org/29321206/diff/29321207/options.html#newcode40
options.html:40: <a class="i18n_options_tab_general"></a>
On 2015/06/29 16:34:11, Thomas Greiner wrote:
> On 2015/06/29 15:59:17, saroyanm wrote:
> > Can you please describe what is the reason of using <a> tags instead of 
> <span>
> > here ?
> > And how are this changes connected with current issue ?
> 
> Semantically, it's a link so we should use the appropriate HTML element for
> that. Furthermore this is a bit of preparation for #2409 to make those items
> focusable.
Fare enough.
> > Also I do think we will need to use ::after Pseudo element for the tab icons
> in
> > future.
> 
> Agree, but that's out of scope for this ticket.
I just commenting to keep in mind.
Will create a ticket for that.

https://codereview.adblockplus.org/29321206/diff/29321207/options.js
File options.js (right):

https://codereview.adblockplus.org/29321206/diff/29321207/options.js#newcode636
options.js:636: ext.backgroundPage.sendMessage({
On 2015/06/29 16:34:11, Thomas Greiner wrote:
> On 2015/06/29 15:59:17, saroyanm wrote:
> > nit: please place the opening curly bracket in the new line for consistency.
> 
> I changed it but seeing that it's done differently in firstRun.js, I'd
actually
> recommend switching to the syntax from there to at least ensure consistency
> within the adblockplusui repository.

Good point, will create ticket for that.

[Felix Dahlke, 2015-06-29 20:24:13]

LGTM