Bug 11411 - Bootstrap theme (enhancement) for (right to left).
Summary: Bootstrap theme (enhancement) for (right to left).
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Karam Qubsi
QA Contact: Testopia
URL:
Keywords:
Depends on: 11601
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-18 08:19 UTC by Karam Qubsi
Modified: 2014-12-07 20:07 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Before the patch (22.54 KB, image/png)
2013-12-18 08:19 UTC, Karam Qubsi
Details
Bug-11411-ENH-Bootstrap-theme-enhancement-for right to left (33.00 KB, patch)
2013-12-18 08:37 UTC, Karam Qubsi
Details | Diff | Splinter Review
After the patch (18.08 KB, image/png)
2013-12-18 09:00 UTC, Karam Qubsi
Details
Bug 11411 [ENH] Bootstrap theme (enhancement) for (right to left) (33.10 KB, patch)
2014-01-01 06:53 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11411 - Bootstrap theme (enhancement) for (right to left) (14.05 KB, patch)
2014-01-30 19:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 11411 - Bootstrap theme (enhancement) for (right to left) (14.16 KB, patch)
2014-02-06 15:49 UTC, Holger Meißner
Details | Diff | Splinter Review
Bug 11411 - Bootstrap theme (enhancement) for (right to left) (11.17 KB, patch)
2014-02-07 14:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11411 [QA Followup] - Tidy css to restore readability (11.17 KB, patch)
2014-02-07 14:13 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Karam Qubsi 2013-12-18 08:19:46 UTC
Created attachment 23615 [details]
Before the patch

Hi,
As you can see in the first attached image the welcome statement and the logout link is located  to the right of the screen, but in the right to left languages it must be to the left.
Comment 1 Karam Qubsi 2013-12-18 08:37:04 UTC Comment hidden (obsolete)
Comment 2 Karam Qubsi 2013-12-18 09:00:31 UTC
Created attachment 23617 [details]
After the patch

The results must be as the attached image .
Comment 3 Chris Cormack 2014-01-01 06:53:12 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2014-01-05 22:31:31 UTC
This works nicely, but not sure if it touches the necessary files. I see there is also /koha-tmpl/opac-tmpl/bootstrap/less/right-to-left.css. Adding Owen in cc as he will know and I hope for some hints :)
Comment 5 Owen Leonard 2014-01-10 15:06:51 UTC
(In reply to Katrin Fischer from comment #4)
> This works nicely, but not sure if it touches the necessary files. I see
> there is also /koha-tmpl/opac-tmpl/bootstrap/less/right-to-left.css.

Yeah there is a better way to handle this, although no one can be blamed for not knowing it because I haven't done a good job of laying out the guidelines.

Two main points to consider:

1. No changes should be made to the files in bootstrap/lib unless they are "official upgrades" to these files, which are all created and maintained outside of the Koha project. If bootstrap-rtl.css has an error it and there isn't an "official upgrade" then the correction needs to be applied in Koha's custom RTL CSS.

2. All CSS in the Bootstrap theme is generated from LESS files (in bootstrap/less), so changes to CSS should be done in the LESS file. There is a specific rtl CSS file there: bootstrap/less/right-to-left.less, where this patch's changes should be made. Then the less file should be compiled and the resulting file put in bootstrap/css.
Comment 6 Katrin Fischer 2014-01-25 08:20:49 UTC
Owen, could you maybe provide a patch here? I am not sure anyone else understands how this works right now (including me) :)
Comment 7 Owen Leonard 2014-01-30 19:21:51 UTC Comment hidden (obsolete)
Comment 8 Holger Meißner 2014-02-06 15:49:16 UTC
Created attachment 25090 [details] [review]
Bug 11411 - Bootstrap theme (enhancement) for (right to left)

This patch improves Koha's layout when a right-to-left language is being
displayed by correcting a float set by the right-to-left version of
Bootstrap CSS.

The patch modifies the right-to-left Less file and includes a
now-minified version of the compiled CSS.

To test, install a right-to-left language like ar-Arab. Apply the patch
and view the OPAC under the Bootstrap theme. When logged in, the
aligned to the left.

Signed-off-by: Holger Meißner <h.meissner.82@web.de>
Comment 9 Holger Meißner 2014-02-06 15:50:03 UTC
Works as described.
Comment 10 Kyle M Hall 2014-02-07 14:13:40 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2014-02-07 14:13:52 UTC Comment hidden (obsolete)
Comment 12 Galen Charlton 2014-02-19 21:40:25 UTC
I have pushed Owen's original patch, but I'm declining to push the follow-ups supplied by Kyle because they defeat the (network performance) purpose of the minification.  Also, in this case I don't think we want the compiled CSS to be readable, since one /should/ be editing the LESS files.

That said, I'm not setting this decision in concrete, as only time will tell whether the benefits of minifying the CSS file outweigh the stumbling blocks for folks using a browser CSS debugger.  However, I will point out that Chrome, at least, includes a pretty-print function in its debugger.

Thanks, Owen!