Skip to content

8260314: Replace border="1" on tables with CSS #2210

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

Closed
wants to merge 5 commits into from

Conversation

aivanov-jdk
Copy link
Member

@aivanov-jdk aivanov-jdk commented Jan 23, 2021

Replace presentational attribute border="1" on <table> element with CSS styles:

table {border: outset 1px}
th, td {border: inset 1px} 

These declarations produce the same rendering as the default one in Firefox and Edge.

Another option is to use solid in both cases.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

Reviewers

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2210/head:pull/2210
$ git checkout pull/2210

@bridgekeeper
Copy link

bridgekeeper bot commented Jan 23, 2021

👋 Welcome back aivanov! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Pull request is ready for review label Jan 23, 2021
@openjdk
Copy link

openjdk bot commented Jan 23, 2021

@aivanov-jdk The following labels will be automatically applied to this pull request:

  • 2d
  • awt
  • swing

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added swing client-libs-dev@openjdk.org 2d client-libs-dev@openjdk.org awt client-libs-dev@openjdk.org labels Jan 23, 2021
@mlbridge
Copy link

mlbridge bot commented Jan 23, 2021

Webrevs

@aivanov-jdk
Copy link
Member Author

@jonathan-gibbons, what do you think?

@mrserb
Copy link
Member

mrserb commented Jan 26, 2021

Probably we can import the CSS used by the javadoc itself?

@aivanov-jdk
Copy link
Member Author

Probably we can import the CSS used by the javadoc itself?

We do. For example, AWT Modality in JDK 15 has borders because of border="1" attribute. If you remove it or change it to border="1" in Developer Tools in browser, the borders around cells disappear.

However, I looked into it further: there are tables in Javadoc comments. One example is AWTKeyStroke constructor which uses class="striped". In most cases, classes in java.desktop module use striped tables.

There's also plain class. I've found one usage of class=plain" in NumericShaper class. This style has collapsed borders around each cell. It looks similar to what we have now in “plain” HTML documents, yet by default the borders are not collapsed, that is you see borders around each individual cell.

The stylesheet also declares borderless class which has no borders.

For the consistent look and feel of documentation, I think striped is the most appropriate class. However, I prefer plain class for componentProperties.html file.

@aivanov-jdk
Copy link
Member Author

@jonathan-gibbons
Copy link
Contributor

In general, I recommend where possible using the styles provided in the standard stylesheet, for overall visual consistency.

FYI, although it seems like the standard styles work for you in this case, if you should ever need it, javadoc now supports package-specific and module-specific stylesheets. Just put *.css files in the doc-files subdirectory or a package or module, and javadoc should pick it up and use it.

@aivanov-jdk
Copy link
Member Author

In general, I recommend where possible using the styles provided in the standard stylesheet, for overall visual consistency.

I totally agree. I overlooked the standard styles for the tables. Thanks to @mrserb for pointing me in the right direction.

Since most tables in java.desktop use striped tables, it makes sense to use this style in these files too.

Although I had said I preferred plain class for componentProperties.html, I changed my opinion after looking at the updated file, it feels lighter when striped.

The table in NumericShaper has confusing rendering with striped class, that's why it uses plain.

FYI, although it seems like the standard styles work for you in this case, if you should ever need it, javadoc now supports package-specific and module-specific stylesheets. Just put *.css files in the doc-files subdirectory or a package or module, and javadoc should pick it up and use it.

Thank you. It's good to know as it allows for keeping consistent look across files as opposed to applying style changes in individual files.

@openjdk
Copy link

openjdk bot commented Jan 26, 2021

@aivanov-jdk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8260314: Replace border="1" on tables with CSS

Reviewed-by: serb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 37 new commits pushed to the master branch:

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Jan 26, 2021
@aivanov-jdk
Copy link
Member Author

/integrate

@openjdk openjdk bot closed this Jan 27, 2021
@openjdk openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 27, 2021
@openjdk
Copy link

openjdk bot commented Jan 27, 2021

@aivanov-jdk Since your change was applied there have been 50 commits pushed to the master branch:

  • e696baa: 8260448: Simplify ManagementFactory$PlatformMBeanFinder
  • b3c8a52: 8259050: Error recovery in lexer could be improved
  • bf15c70: 8260460: GitHub actions still fail on Linux x86_32 with "Could not configure libc6:i386"
  • 3e4194c: 8260022: [ppc] os::print_function_and_library_name shall resolve function descriptors transparently
  • fa40a96: 8253420: Refactor HeapRegionManager::find_highest_free
  • 4d004c9: 8260449: Remove stale declaration of SATBMarkQueue::apply_closure_and_empty()
  • fd2641e: 8260236: better init AnnotationCollector _contended_group
  • 1c77046: 8260404: jvm_io.h include missing in a number of files
  • bd2744d: 8260106: Shenandoah: refactor reference updating closures and related code
  • c836da3: 8252412: [macos11] system dynamic libraries removed from filesystem
  • ... and 40 more: https://git.openjdk.java.net/jdk/compare/f624dba612b79605f2b71c9d0be71dc818f885f2...master

Your commit was automatically rebased without conflicts.

Pushed as commit 7ed591c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@aivanov-jdk aivanov-jdk deleted the JDK-8260314 branch February 18, 2021 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2d client-libs-dev@openjdk.org awt client-libs-dev@openjdk.org integrated Pull request has been integrated swing client-libs-dev@openjdk.org
Development

Successfully merging this pull request may close these issues.

3 participants