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

Improve table semantics using thead and tbody instead of a standard row as header #675

Merged
merged 2 commits into from Mar 11, 2016

Conversation

flodolo
Copy link
Collaborator

@flodolo flodolo commented Mar 11, 2016

Strip tags in unlocalized view, that also improves results (e.g. for words like "tag", "file", etc.).
Also fixes #670

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @pascalchevrel, @TheoChevalier and @tx2z to be potential reviewers


$section_title = "Strings that have changed significantly in English between {$repo_one} and {$repo_two} but for which the entity name didn’t change";
$table = "
<p class='section_title'>{$section_title}</p>
Copy link
Member

Choose a reason for hiding this comment

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

If it's a title maybe it should use a tag ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not completely sure: it's not an actual title, more a subtitle, and before it was even more confusing (part of the table). The actual title would be the one above (linked as an anchor).

schermata 2016-03-11 alle 13 54 01

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(it might make sense to change the class name, e.g. section_description or similar)

Copy link
Member

Choose a reason for hiding this comment

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

or many a th that would span all columns if it's part of the table, or a caption tag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me check how <caption> is displayed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tried but it would require quite a few changes, especially for smaller viewports (it breaks quite badly), and I'm not completely convinced that it should be part of the table.

@flodolo
Copy link
Collaborator Author

flodolo commented Mar 11, 2016

Updated variable/CSS class name in a separate commit. Not sure if we should drop the bold too.

pascalchevrel added a commit that referenced this pull request Mar 11, 2016
Improve table semantics using thead and tbody instead of a standard row as header
@pascalchevrel pascalchevrel merged commit 5d3cc01 into mozfr:master Mar 11, 2016
@pascalchevrel
Copy link
Member

oops, missed that it wasn't squashed yet :(

@flodolo flodolo deleted the issue670 branch March 11, 2016 14:08
@flodolo flodolo mentioned this pull request Mar 14, 2016
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.

[unlocalized strings] Sanitize English words before displaying, fix first row style
3 participants