Bug 17942 - Update style of the web installer with Bootstrap 3
Summary: Update style of the web installer with Bootstrap 3
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 18462 (view as bug list)
Depends on: 17855
Blocks:
  Show dependency treegraph
 
Reported: 2017-01-19 13:57 UTC by Owen Leonard
Modified: 2017-05-26 12:39 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17942 - Update style of the web installer with Bootstrap 3 (135.85 KB, patch)
2017-01-19 19:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17942 - Update style of the web installer with Bootstrap 3 (135.80 KB, patch)
2017-01-27 19:27 UTC, Owen Leonard
Details | Diff | Splinter Review
Staff is black (161.68 KB, image/jpeg)
2017-01-27 20:09 UTC, M. Tompsett
Details
Superlibrarian is red (171.91 KB, image/jpeg)
2017-01-27 20:09 UTC, M. Tompsett
Details
WebIntallerStep1 (29.30 KB, image/png)
2017-01-27 20:41 UTC, Josef Moravec
Details
[SIGNED-OFF] Bug 17942 - Update style of the web installer with Bootstrap 3 (135.81 KB, patch)
2017-02-01 08:03 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17942 - Update style of the web installer with Bootstrap 3 (126.29 KB, patch)
2017-02-09 22:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17942 - follow up - add anticlickjack style id to remove js error (1.35 KB, patch)
2017-02-10 01:31 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 17942 - Update style of the web installer with Bootstrap 3 (126.34 KB, patch)
2017-02-10 01:33 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 17942 - follow up - add anticlickjack style id to remove js error (1.35 KB, patch)
2017-02-10 01:34 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 17942 - Add anti-clickjack code to installer doc head close (867 bytes, patch)
2017-02-21 15:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17942 - Add anti-clickjack code to installer doc head close (1.17 KB, patch)
2017-02-27 12:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17942 - Update style of the web installer with Bootstrap 3 (116.33 KB, patch)
2017-04-28 15:48 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17942 - Add anti-clickjack code to installer doc head close (1.28 KB, patch)
2017-04-28 15:48 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17942 - Update style of the web installer with Bootstrap 3 (116.48 KB, patch)
2017-04-28 18:47 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17942 - Add anti-clickjack code to installer doc head close (1.41 KB, patch)
2017-04-28 18:48 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17942 [Follow-up] Update style of the web installer with Bootstrap 3 (12.43 KB, patch)
2017-05-03 13:18 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17942 - Update style of the web installer with Bootstrap 3 (116.55 KB, patch)
2017-05-03 15:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17942 - Add anti-clickjack code to installer doc head close (1.48 KB, patch)
2017-05-03 15:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17942: Remove type for messages (4.87 KB, patch)
2017-05-03 15:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17942 [Follow-up] Update style of the web installer with Bootstrap 3 (12.50 KB, patch)
2017-05-03 15:01 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2017-01-19 13:57:11 UTC
I'm working on a markup and style overhaul of the web installer and the new onboarding templates. One preliminary screenshot:  http://zivotdesign.com/p/#1/14847560123053

I'm hoping that this patch and those for Bug 17855 can be (eventually) pushed at the same time.
Comment 1 Owen Leonard 2017-01-19 19:21:12 UTC Comment hidden (obsolete)
Comment 2 M. Tompsett 2017-01-19 20:26:42 UTC
Why is "select all data" a link and not a button?
Next why are the texts staff and superlibrarian (different pages) differing styles?
Don't you love the set koha version text? :(
Comment 3 Owen Leonard 2017-01-23 13:20:37 UTC
(In reply to M. Tompsett from comment #2)
> Why is "select all data" a link and not a button?

Because everywhere else in Koha "select all" is a link.

> Next why are the texts staff and superlibrarian (different pages) differing
> styles?

Since I'm not following along with you I don't know what screen you're looking at when you make comments. Can you please be specific about where you're seeing these issues?

> Don't you love the set koha version text? :(

Can you please explain this comment? Are you making a suggestion? Pointing out a mistake?
Comment 4 M. Tompsett 2017-01-23 15:42:36 UTC
(In reply to Owen Leonard from comment #3)
> Because everywhere else in Koha "select all" is a link.

Hmmm... it wasn't previously, but I see that has changed.
Could we style it pretty like:
<a href="#" id="select_all"><i class="fa fa-check"></i> Select all</a>
(grabbed from /cgi-bin/koha/members/members-home.pl)

No biggie on this.


> Since I'm not following along with you I don't know what screen you're
> looking at when you make comments. Can you please be specific about where
> you're seeing these issues?

With just 17855 applied:
Create a new patron category (staff)
Create koha administrator patron (superlibrarian)

With this applied as well:
Web installer › Create a patron category (staff)
Web installer › Create Koha administrator patron (superlibrarian)

Is there a reason to have them differing styles? I sort of expected them the same.
Comment 5 Owen Leonard 2017-01-23 19:26:03 UTC
Sorry, I still don't understand your comments.
Comment 6 M. Tompsett 2017-01-24 01:43:36 UTC
(In reply to Owen Leonard from comment #5)
> Sorry, I still don't understand your comments.

I gave the titles of the pages (and what is on them).
Staff is like normal black texty-text, and superlibrarian is like square red blocky text. I'll attach a screen shot if I must, but I thought running the web installer and looking for the text in question on the pages listed would be evident. I'm sorry if my descriptive abilities are lacking.
Comment 7 Owen Leonard 2017-01-24 13:13:34 UTC
Okay, I think I understand. I think it's fine.

> Don't you love the set koha version text? :(

Can you explain this comment please?
Comment 8 Josef Moravec 2017-01-25 16:46:26 UTC
Just read quickly through the code.

First I have to say: good job Owen!

Just one think:
It is enough to use container-fluid for full width grid, row-fluid should be just "row" acording to bootstrap documentation
Comment 9 M. Tompsett 2017-01-25 17:04:59 UTC
(In reply to Owen Leonard from comment #7)
> Okay, I think I understand. I think it's fine.
> 
> > Don't you love the set koha version text? :(
> 
> Can you explain this comment please?

That was part of the transitional changes on 17855. Nevermind.
Comment 10 Owen Leonard 2017-01-27 19:27:07 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2017-01-27 19:28:00 UTC
(In reply to Josef Moravec from comment #8)
> It is enough to use container-fluid for full width grid, row-fluid should be
> just "row" acording to bootstrap documentation

Quite right. It is fixed now.
Comment 12 M. Tompsett 2017-01-27 20:09:23 UTC
Created attachment 59624 [details]
Staff is black
Comment 13 M. Tompsett 2017-01-27 20:09:51 UTC
Created attachment 59625 [details]
Superlibrarian is red
Comment 14 M. Tompsett 2017-01-27 20:10:35 UTC
There, screen shots of what I was talking about. Explain the reasons for the difference.
Comment 15 Josef Moravec 2017-01-27 20:41:19 UTC Comment hidden (obsolete)
Comment 16 Josef Moravec 2017-01-27 20:46:28 UTC
Hm, maybe some fa icon by link "Select all sample data" would be useful to make it more visible...

I like the using of "btn-primary" ;)
Comment 17 Josef Moravec 2017-01-27 20:48:17 UTC
I faced a weird behavior, after installation, I was redirected to onboarding step 1, then immediatelly to step 2 and then step 3... so imposible to use two first forms/steps.
Comment 18 Josef Moravec 2017-01-27 20:55:20 UTC Comment hidden (obsolete)
Comment 19 M. Tompsett 2017-01-27 22:05:18 UTC
Don't sign off until I get feedback on comment #14.
Comment 20 Owen Leonard 2017-01-30 13:43:37 UTC
(In reply to M. Tompsett from comment #14)
> There, screen shots of what I was talking about. Explain the reasons for the
> difference.

This is rude.

(In reply to M. Tompsett from comment #19)
> Don't sign off until I get feedback on comment #14.

I don't think this comment is appropriate.

"Superlibrarian" is red because it is highlighted for visibility in a page which is structured with sections topped by headings. There is nothing in that section other than text. I think its appropriate to provide a little more visibility to that text to help show that it is the focus of that section.

"Staff" is plain black because it is informational text being displayed as part of a form. As in other places in Koha, when a line in a form displays information instead of some kind of input, plain text is paired with <span class="label">Label:</label> to preserve the structure and flow of the form. The plain text fits with the structure of the form and does not need to be highlighted.
Comment 21 M. Tompsett 2017-02-01 02:59:42 UTC
(In reply to Owen Leonard from comment #20)
> (In reply to M. Tompsett from comment #14)
> > There, screen shots of what I was talking about. Explain the reasons for the
> > difference.
> 
> This is rude.

I apologize for the shortness. I'm sorry. I was wrong.


> (In reply to M. Tompsett from comment #19)
> "Superlibrarian" is red because it is highlighted for visibility in a page
> which is structured with sections topped by headings. There is nothing in
> that section other than text. I think its appropriate to provide a little
> more visibility to that text to help show that it is the focus of that
> section.
> 
> "Staff" is plain black because it is informational text being displayed as
> part of a form. As in other places in Koha, when a line in a form displays
> information instead of some kind of input, plain text is paired with <span
> class="label">Label:</label> to preserve the structure and flow of the
> form.
> The plain text fits with the structure of the form and does not need to be
> highlighted.

Thank you for the explanation.
Comment 22 M. Tompsett 2017-02-01 03:04:12 UTC
(In reply to Josef Moravec from comment #18)
> > In English it's OK

I can confirm this. Though, OK is so bland. It is quite pretty. Did you get it working in Czech, Josef? If so, would you like to sign off, or shall I?
Comment 23 Josef Moravec 2017-02-01 08:03:41 UTC Comment hidden (obsolete)
Comment 24 Josef Moravec 2017-02-01 08:05:14 UTC
(In reply to M. Tompsett from comment #22)
> (In reply to Josef Moravec from comment #18)
> > > In English it's OK
> 
> I can confirm this. Though, OK is so bland. It is quite pretty. Did you get
> it working in Czech, Josef? If so, would you like to sign off, or shall I?

Yes, after regenerating templates everything is OK, so my mistake...
Comment 25 Josef Moravec 2017-02-01 08:06:38 UTC
Signed off, has to say it looks really nice! Good job Owen!
Comment 26 M. Tompsett 2017-02-03 04:56:24 UTC
Sorry, Owen. Looks like 17855's cleanup to address the points JD requires a rebase here.
Comment 27 Katrin Fischer 2017-02-09 22:44:01 UTC Comment hidden (obsolete)
Comment 28 Katrin Fischer 2017-02-09 22:46:22 UTC
Tried to resolve the conflicts - nothing explodes in an obvious way. Conflict files were:
 koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep2.tt
 koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep3.tt
Comment 29 Liz Rea 2017-02-10 01:31:15 UTC Comment hidden (obsolete)
Comment 30 Liz Rea 2017-02-10 01:33:18 UTC
Created attachment 60100 [details] [review]
Bug 17942 - Update style of the web installer with Bootstrap 3

This patch makes style and markup changes to the web installer and new
onboarding tool (Bug 17855).

- Markup has been corrected, improved, and in many places reindented.
- Some page titles have been made more specific.
- Some JavaScript and CSS have been moved to separate files.

In the onboarding tool I have removed form validation from the markup
and JavaScript in many cases where the requirements were not matched
elsewhere in Koha. For instance, we shouldn't limit item type
descriptions to only letters because the database doesn't require such a
limit.

To test, apply the patch and run the web installer with an empty
database. Confirm that the installation process completes correctly and
that each page looks good and works correctly.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Comment 31 Liz Rea 2017-02-10 01:34:55 UTC Comment hidden (obsolete)
Comment 32 Liz Rea 2017-02-10 01:35:51 UTC
I also checked this in other languages, as long as the languages have been regenerated all looks just great.

Finally, our installer looks like the modern web! So exciting!
Comment 33 Alex Buckley 2017-02-17 03:00:17 UTC
I have tested bug 17855 (and obsoleted the old patches), bug 18039 and this bug together. 

One thing I would like to suggest is that the IF statement check for the existence of libraries, categories and item types in the onboardingstep1.tt, onboardingstep2, and onboardingstep4.tt files is moved out of the head element and back down each file to where it was originally written in bug 17855.

I am happy to do a commit of this.

The reason I am suggesting it is the redirection in the head element is not immediate and so the user has the form to create a library, patron category, item type displayed for a few seconds before they are redirected. This could be rather confusing for new users. 

By having the library,category,itemtype check as part of a longer if statement that had the elseif check of 'if add_validate_<library,category or itemtype>' was true. Then the form would not be displayed whilst redirection was loading the next appropriate form.

I am writing this to find out if there will be any objections to me making this change
Comment 34 Kyle M Hall 2017-02-21 15:25:33 UTC Comment hidden (obsolete)
Comment 35 Kyle M Hall 2017-02-27 12:52:34 UTC
Created attachment 60710 [details] [review]
Bug 17942 - Add anti-clickjack code to installer doc head close
Comment 36 Jonathan Druart 2017-03-29 15:32:13 UTC
Blocked by bug 17855.
Comment 37 Owen Leonard 2017-04-26 17:32:56 UTC
This work has been moved to Bug 18462.

*** This bug has been marked as a duplicate of bug 18462 ***
Comment 38 Owen Leonard 2017-04-28 15:48:39 UTC
Created attachment 62856 [details] [review]
Bug 17942 - Update style of the web installer with Bootstrap 3

This patch makes style and markup changes to the web installer and new
onboarding tool (Bug 17855).

- Markup has been corrected, improved, and in many places reindented.
- Some page titles have been made more specific.
- Some JavaScript and CSS have been moved to separate files.

In the onboarding tool I have removed form validation from the markup
and JavaScript in many cases where the requirements were not matched
elsewhere in Koha. For instance, we shouldn't limit item type
descriptions to only letters because the database doesn't require such a
limit.

To test, apply the patch and run the web installer with an empty
database. Confirm that the installation process completes correctly and
that each page looks good and works correctly.
Comment 39 Owen Leonard 2017-04-28 15:48:43 UTC
Created attachment 62857 [details] [review]
Bug 17942 - Add anti-clickjack code to installer doc head close
Comment 40 Marc Véron 2017-04-28 18:47:11 UTC
Created attachment 62861 [details] [review]
Bug 17942 - Update style of the web installer with Bootstrap 3

This patch makes style and markup changes to the web installer and new
onboarding tool (Bug 17855).

- Markup has been corrected, improved, and in many places reindented.
- Some page titles have been made more specific.
- Some JavaScript and CSS have been moved to separate files.

In the onboarding tool I have removed form validation from the markup
and JavaScript in many cases where the requirements were not matched
elsewhere in Koha. For instance, we shouldn't limit item type
descriptions to only letters because the database doesn't require such a
limit.

To test, apply the patch and run the web installer with an empty
database. Confirm that the installation process completes correctly and
that each page looks good and works correctly.

Works as advertised
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 41 Marc Véron 2017-04-28 18:48:05 UTC
Created attachment 62862 [details] [review]
Bug 17942 - Add anti-clickjack code to installer doc head close

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 42 Alex Buckley 2017-04-28 23:25:03 UTC
*** Bug 18462 has been marked as a duplicate of this bug. ***
Comment 43 Jonathan Druart 2017-05-02 21:24:10 UTC
1/ #login_error is not styled, is it expected?

2/ We talked about the default style of btn-primary, should not we apply it a custom style?

3/ "Select no mandatory data", "Select all mandatory data" and others will not be translatable. I think it would be better to list all of them in the template.
Note that we should only use "mandatory" and "optional" should be allowed to simplify the structure of our installer files.

4/ TODO later, the JS checks for password should use password_check.inc from bug 18298 (not pushed yet).

5/ Not blocker - "Enrolment period: " block is not really nice ;)

6/ "Enrolment Period" > "Until date"
Date is like "MM/DD/YYYY", but hint says "(YYYY-MM-DD)"

7/ There are too many checks defined in onboarding.js, please double... check!
(cardnumber_check for instance)
Comment 44 Owen Leonard 2017-05-03 13:18:08 UTC
Created attachment 63013 [details] [review]
Bug 17942 [Follow-up] Update style of the web installer with Bootstrap 3

This patch makes changes following QA comments:

- Login error messages now have a style
- Select all/Select none links should now be translatable
- Date format hint now correctly detects date format preference
- Unused JS is removed from onboarding.js

This patch also modifies the 'category_code_check' validation method to
allow numbers as well as letters.
Comment 45 Owen Leonard 2017-05-03 13:21:34 UTC
(In reply to Jonathan Druart from comment #43)
> 2/ We talked about the default style of btn-primary, should not we apply it
> a custom style?

I think this is a good idea but at this point I think it should be addressed in a follow-up.
 
> 3/ "Select no mandatory data", "Select all mandatory data" and others will
> not be translatable.

I hope it is acceptable now.

> Note that we should only use "mandatory" and "optional" should be allowed to
> simplify the structure of our installer files.

I don't understand this comment.

> 5/ Not blocker - "Enrolment period: " block is not really nice ;)

I agree, but I think this will have to wait for a follow-up.
Comment 46 Jonathan Druart 2017-05-03 15:00:54 UTC
Created attachment 63019 [details] [review]
Bug 17942 - Update style of the web installer with Bootstrap 3

This patch makes style and markup changes to the web installer and new
onboarding tool (Bug 17855).

- Markup has been corrected, improved, and in many places reindented.
- Some page titles have been made more specific.
- Some JavaScript and CSS have been moved to separate files.

In the onboarding tool I have removed form validation from the markup
and JavaScript in many cases where the requirements were not matched
elsewhere in Koha. For instance, we shouldn't limit item type
descriptions to only letters because the database doesn't require such a
limit.

To test, apply the patch and run the web installer with an empty
database. Confirm that the installation process completes correctly and
that each page looks good and works correctly.

Works as advertised
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 47 Jonathan Druart 2017-05-03 15:01:02 UTC
Created attachment 63020 [details] [review]
Bug 17942 - Add anti-clickjack code to installer doc head close

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 48 Jonathan Druart 2017-05-03 15:01:09 UTC
Created attachment 63021 [details] [review]
Bug 17942: Remove type for messages

It it no longer in used by the templates.

Also add the include files for step5 (if the circ rule is not inserted)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 49 Jonathan Druart 2017-05-03 15:01:14 UTC
Created attachment 63022 [details] [review]
Bug 17942 [Follow-up] Update style of the web installer with Bootstrap 3

This patch makes changes following QA comments:

- Login error messages now have a style
- Select all/Select none links should now be translatable
- Date format hint now correctly detects date format preference
- Unused JS is removed from onboarding.js

This patch also modifies the 'category_code_check' validation method to
allow numbers as well as letters.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 50 Kyle M Hall 2017-05-12 14:26:56 UTC
Pushed to master for 17.05, thanks Owen, Jonathan!
Comment 51 Katrin Fischer 2017-05-14 10:18:55 UTC
Depends on bug 18462 which is not in 16.11.x (new feature for 17.05)
Comment 52 Katrin Fischer 2017-05-14 10:33:05 UTC
Actually depends on bug 17855!