Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Html validation #13

Closed
wants to merge 6 commits into from
Closed

Html validation #13

wants to merge 6 commits into from

Conversation

ddrury
Copy link
Contributor

@ddrury ddrury commented Oct 22, 2013

Found this while checking HTML validation following the update of CKEditor (not yet finished) in function edit_language_checkboxes()

@fisharebest
Copy link
Owner

I'm not sure this is the right fix.

The second switch block should be creating empty table cells, so that every row contains exactly 3 elements. Perhaps the fix is to change this

switch ($i % 3) {
case 0: echo '</tr>'; break;
case 1: echo '</td></td></tr>'; break;
case 2: echo '</td></tr>'; break;
}

into this:

switch ($i % 3) {
case 0: break;
case 1: echo '<td></td><td></td></tr>'; break;
case 2: echo '<td></td></tr>'; break;
}

@ddrury
Copy link
Contributor Author

ddrury commented Oct 22, 2013

Of course - sorry

You doing it?

Regards
David
Please consider the environment before printing this e-mail

------ Original Message ------
From: "Greg Roach" notifications@github.com
To: "fisharebest/webtrees" webtrees@noreply.github.com
Cc: "David Drury" david@drury.me.uk
Sent: 22/10/2013 14:33:42
Subject: Re: [webtrees] Html validation (#13)

I'm not sure this is the right fix.

The second switch block should be creating empty table cells, so that
every row contains exactly 3

elements. Perhaps the fix is to change this
switch ($i % 3) { case 0: echo ''; break; case 1: echo
''; break; case 2: echo ''; break; }
into this:

switch ($i % 3) { case 0: break; case 1: echo
''; break; case 2: echo ''; break;
}

Reply to this email directly or view it on GitHub.

@fisharebest
Copy link
Owner

My lunch break is over, so if you have the time to check this, go ahead...

@ddrury
Copy link
Contributor Author

ddrury commented Oct 23, 2013

Ok,

done plus also added changes as discussed on forum to stories module.php

@fisharebest
Copy link
Owner

I've applied these changes. Thanks.

TIP: you can merge multiple commits into one (git rebase --interactive). This makes it easier to review the changes.

In this case, I merged the first 4 into one, which is probably what you would have done.

@ddrury
Copy link
Contributor Author

ddrury commented Oct 24, 2013

Hi Greg,

Thanks for info re rebase - I'm afraid it's still pretty much hit and
miss with me and GIT at the moment!

Two questions:

  1. what is the official method of contacting you for advice etc. prior
    to submitting work.
  2. I've attached a screen shot of CKEditor v4.2.2 running. You will see
    that the only official skin with this version is 'moono' which is
    monochrome and supposedly good for accessibility but doesn't look quite
    so nice as the current version. Is this acceptable to you? (there is an
    unofficial skin called 'kama' which is moono with coloured icons)

I promise I won't contact you unless absolutely necessary

Regards
David
Please consider the environment before printing this e-mail

------ Original Message ------
From: "Greg Roach" notifications@github.com
To: "fisharebest/webtrees" webtrees@noreply.github.com
Cc: "David Drury" david@drury.me.uk
Sent: 23/10/2013 21:42:53
Subject: Re: [webtrees] Html validation (#13)

I've applied these changes. Thanks.

TIP: you can merge multiple commits into one (git rebase
--interactive). This makes it easier to review the changes.

In this case, I merged the first 4 into one, which is probably what you
would have done.


Reply to this email directly or view it on GitHub.

@webtrees-pesz webtrees-pesz mentioned this pull request Aug 18, 2023
@makitso makitso mentioned this pull request Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants