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.

Sign up for GitHub

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

Jump to bottom

Add HTMLTableCellElement.{align|vAlign} #32993

Merged
merged 27 commits into from
Apr 10, 2024
Merged

Add HTMLTableCellElement.{align|vAlign} #32993

merged 27 commits into from
Apr 10, 2024

Conversation

teoli2003
Copy link
Member

Description

This PR adds docs for the two properties:

  • HTMLTableCellElement.align
  • HTMLTableCellElement.valign

Motivation

These properties are supported by all engines.

Though deprecated, they are common tasks for beginners: we need documentation that points to the right way of doing this (TM), using text-align and vertical-align

Additional details

There is no example as this is deprecated: the example sections point to examples using the modern (CSS) way of doing it so that they are one click away.

Related issues and pull requests

It is part of mdn/mdn#520

@teoli2003 teoli2003 requested review from a team as code owners April 8, 2024 07:42
@teoli2003 teoli2003 requested review from wbamberg and chrisdavidmills and removed request for a team April 8, 2024 07:42
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs Content:WebAPI Web API docs size/m 51-500 LoC changed labels Apr 8, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 8, 2024 •
edited

Preview URLs

Flaws (13)

Note! 3 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/HTMLTableCellElement
Title: HTMLTableCellElement
Flaw count: 13

  • macros:
    • /en-US/docs/Web/API/HTMLTableCellElement/abbr does not exist
    • /en-US/docs/Web/API/HTMLTableCellElement/cellIndex does not exist
    • /en-US/docs/Web/API/HTMLTableRowElement/cells does not exist
    • /en-US/docs/Web/API/HTMLTableCellElement/colSpan does not exist
    • /en-US/docs/Web/API/HTMLTableCellElement/headers does not exist
    • and 8 more flaws omitted

(comment last updated: 2024-04-10 15:01:55)

@teoli2003 teoli2003 requested a review from estelle April 8, 2024 07:47
@chrisdavidmills chrisdavidmills removed their request for review April 8, 2024 10:14
estelle
estelle reviewed Apr 8, 2024
View reviewed changes
Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

I realize these are deprecated, but they both get and set. The "get" any value, even if not valid, with empty string if not present.

files/en-us/web/api/htmltablecellelement/align/index.md Outdated

{{APIRef("HTML DOM")}}{{deprecated_header}}

The **`HTMLTableCellElement.align`** property is a string indicating how to horizontally align text in the cell.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The **`HTMLTableCellElement.align`** property is a string indicating how to horizontally align text in the cell.
The **`HTMLTableCellElement.align`** property is a string indicating how to horizontally align text in the {{htmlelement("th")}} or {{htmlelement("td")}} table cell.

we should include the two elements on which it is used as links. Not sure about having to mention "table", or linking to that.

also, it's a setter, but not a getter. should we mention that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Commited.

According to the IDL it is both a setter and a getter. (But it ignores the value of text-align of course). I don't think it needs more info here.

files/en-us/web/api/htmltablecellelement/align/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/htmltablecellelement/align/index.md Outdated

The **`HTMLTableCellElement.align`** property is a string indicating how to horizontally align text in the cell.

**Note:** This property is deprecated and CSS should be used to horizontally align text in a cell. Use the {{cssxref("text-align")}} property instead.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
**Note:** This property is deprecated and CSS should be used to horizontally align text in a cell. Use the {{cssxref("text-align")}} property instead.
**Note:** This property is deprecated. Use the CSS {{cssxref("text-align")}} property, which takes precedence, to horizontally align text in a cell instead.

we should mention that it this JS has no effect if this property is set. this is my way of suggesting it. you may have a better suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

files/en-us/web/api/htmltablecellelement/align/index.md Show resolved Hide resolved
files/en-us/web/api/htmltablecellelement/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/htmltablecellelement/valign/index.md Outdated

## Value

The possible values are:
Copy link
Member

Choose a reason for hiding this comment

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

Pretty sure the values were top, bottom, center, middle, and baseline. IE5 supported justify as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you are right. Fixing. We don't need to list justify anymore, though.

files/en-us/web/api/htmltablecellelement/valign/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-align/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-align/index.md Show resolved Hide resolved
files/en-us/web/css/text-align/index.md Outdated Show resolved Hide resolved
@wbamberg wbamberg removed their request for review April 10, 2024 06:24
teoli2003 and others added 13 commits April 10, 2024 10:29
@teoli2003 @skyclouds2001
Update files/en-us/web/api/htmltablecellelement/align/index.md
bf51300 
Co-authored-by: skyclouds2001 <[email protected]>
@teoli2003 @skyclouds2001
Update files/en-us/web/api/htmltablecellelement/align/index.md
ddb5ac6 
Co-authored-by: skyclouds2001 <[email protected]>
@teoli2003 @skyclouds2001
Update files/en-us/web/api/htmltablecellelement/valign/index.md
56464b4 
Co-authored-by: skyclouds2001 <[email protected]>
@teoli2003 @skyclouds2001
Update files/en-us/web/api/htmltablecellelement/valign/index.md
b6ca1c0 
Co-authored-by: skyclouds2001 <[email protected]>
@teoli2003 @estelle
Update files/en-us/web/css/text-align/index.md
cd8fa93 
Co-authored-by: Estelle Weyl <[email protected]>
@teoli2003 @estelle
Update files/en-us/web/api/htmltablecellelement/align/index.md
1c94cc9 
Co-authored-by: Estelle Weyl <[email protected]>
@teoli2003 @estelle
Update files/en-us/web/api/htmltablecellelement/align/index.md
e204365 
Co-authored-by: Estelle Weyl <[email protected]>
@teoli2003
Update files/en-us/web/api/htmltablecellelement/align/index.md
df4ea50
@teoli2003
Update files/en-us/web/api/htmltablecellelement/align/index.md
7428ff7
@teoli2003
Update index.md
7aa0f53
@teoli2003 @estelle
Update files/en-us/web/css/text-align/index.md
afd7c19 
Co-authored-by: Estelle Weyl <[email protected]>
@teoli2003 @estelle
Update files/en-us/web/api/htmltablecellelement/index.md
605556f 
Co-authored-by: Estelle Weyl <[email protected]>
@teoli2003 @estelle
Update files/en-us/web/api/htmltablecellelement/valign/index.md
f26b0ed 
Co-authored-by: Estelle Weyl <[email protected]>
@teoli2003
Copy link
Member Author

I think I have addressed (or answered) all points. Ready for a second pass.

@teoli2003
Copy link
Member Author

(The linting failing test seems bogus. There are no errors listed, and I pushed from local, so the linter ran locally too (without error)

@teoli2003 teoli2003 requested a review from estelle April 10, 2024 13:00
@estelle
Copy link
Member

estelle commented Apr 10, 2024

(The linting failing test seems bogus. There are no errors listed, and I pushed from local, so the linter ran locally too (without error)

I think the error was a simple space missing. If it's going to error, it should list the error. I found it by looking at the commit that first produced the green X. committing the space addition seems to have brought back the green x.

estelle
estelle approved these changes Apr 10, 2024
View reviewed changes
Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

a few edits, but approving to not hold you up.

files/en-us/web/api/htmltablecellelement/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/htmltablecellelement/valign/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/htmltablecellelement/valign/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/htmltablecellelement/valign/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/text-align/index.md Outdated Show resolved Hide resolved
@teoli2003
Copy link
Member Author

Thank you, @estelle!

@teoli2003 teoli2003 merged commit 6c858b4 into mdn:main Apr 10, 2024
8 checks passed
This was referenced Apr 11, 2024
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions left review comments

@skyclouds2001 skyclouds2001 skyclouds2001 left review comments

@estelle estelle estelle approved these changes

Assignees
No one assigned
Labels
Content:CSS Cascading Style Sheets docs Content:WebAPI Web API docs size/m 51-500 LoC changed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@teoli2003 @estelle @skyclouds2001