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

Add hotlinking anchor tags to each table header #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Kamori
Copy link

@Kamori Kamori commented Oct 4, 2021

This allows you to send urls like http://127.0.0.1:5500/index.html#table.shopping to your friend, and have them pointed directly to the shopping table.

Thanks for making this.

ps, my vscode is trying to make a lot of formatting changes. Perhaps either linting the html or providing a linting config would be appreciated :)

@Kamori Kamori changed the title Add hotlinking anchor tags to each table headers Add hotlinking anchor tags to each table header Oct 4, 2021
@Kamori Kamori mentioned this pull request Oct 4, 2021
@Kamori
Copy link
Author

Kamori commented Oct 5, 2021

Screen Shot 2021-10-04 at 7 12 36 PM

I've updated it to instead use table of contents, rather than hotlinking the <th> tags.

This also moves the <script> to the bottom of the html. This way the renderer will parse the entire html block first, and then the javascript. I've also taken it off the <body onload=""> as this was causing the page to "refresh" when onload finished, everytime.

@Michaelangel007
Copy link
Owner

Michaelangel007 commented Oct 5, 2021

I don't use a linter since I custom format all the html for readability.

i.e. I'll align columns with extra whitespace, insert <meta/> for alignment, etc.

The sidebar TOC is a great idea!

I'm currently in the middle of adding a gem table but I'll (hopefully) take a look at this later tonight once I verify the 1-column and 2-column mode work correctly across desktop and mobile. I test resizing the browser window width on these monitors/resolutions and platforms:

  • desktop 2560x1440
  • desktop PC 2160x3840
  • mobile iOS

Thanks for the suggestion of moving the Javascript to the bottom.

The hack of using a timer shouldn't be needed -- that's what onLoad is for but I'll take a look if that provides a better solution.

@Kamori
Copy link
Author

Kamori commented Oct 5, 2021

Appreciate ya!

The TOC won't support resizing as is :( Mainly because I used a fixed location settings in CSS. I could redo this to support resizing, just would take more time.

I don't use a linter since I custom format all the html for readability.
i.e. I'll align columns with extra whitespace, insert for alignment, etc.

For me personally, this made it harder to assist :( But, its your project I am more than happy to adhere to your style :)

And as for

The hack of using a timer shouldn't be needed -- that's what onLoad is for but I'll take a look if that provides a better solution.

Yea the only reason I did this is because of how the renderer loads the dom. Since I was injecting hash tags, I needed more control over the order the process was ran in. Since I moved to the Table of Contents view, that became less necessary to do. We can remove that change if we want to.

@Michaelangel007
Copy link
Owner

Gem table finally got added so I can circle around to this tonight. I'll probably clone your repo and do some 1 and 2 column testing. My main concern is making sure a screen width of 1920 looks good.

Only deal breaker right now (trivial as it is) is the brace style. I use Allman style. Please fix that.

I'm actually excited about this PR as this make linking SO much more user friendly -- for others and myself! :-)

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.

2 participants