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 <time> element #86

Merged
merged 9 commits into from
May 9, 2023
Merged

Add <time> element #86

merged 9 commits into from
May 9, 2023

Conversation

harrydayexe
Copy link
Contributor

This is a quick implementation of the <time> element.

Reference: https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-time-element

Copy link
Owner

@JohnSundell JohnSundell left a comment

Choose a reason for hiding this comment

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

Thanks for this @harrydayexe, and very sorry for taking so long to review this PR! I left a suggestion inline, which I think might simplify the implementation quite a bit (and make it easier to maintain). Let me know what you think.

Sources/Plot/API/HTMLAttributes.swift Outdated Show resolved Hide resolved
@JohnSundell JohnSundell added the awaiting update Waiting for an update from the PR's author. label Mar 31, 2023
This is to remvove a dependecy on Foundation for a DateFormatter and to
enable the user to select what kind of format they wish to select due to
the multitude of formats available
Copy link
Owner

@JohnSundell JohnSundell left a comment

Choose a reason for hiding this comment

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

Thanks for updating your PR, @harrydayexe! I just have a few more pieces of feedback in order to make this change fully mergable. Let me know if I can help out in any way.

Sources/Plot/API/HTML.swift Outdated Show resolved Hide resolved
Sources/Plot/API/HTMLAttributes.swift Outdated Show resolved Hide resolved
Sources/Plot/API/HTMLAttributes.swift Show resolved Hide resolved
Sources/Plot/API/HTMLComponents.swift Show resolved Hide resolved
Sources/Plot/API/HTMLElements.swift Outdated Show resolved Hide resolved
Tests/PlotTests/HTMLComponentTests.swift Outdated Show resolved Hide resolved
Removed Foundation imports and unneeded HTMLDateTimeContext as well as fixed some
code style issues
@harrydayexe
Copy link
Contributor Author

Hopefully those changes are everything you spotted!

@JohnSundell JohnSundell removed the awaiting update Waiting for an update from the PR's author. label May 9, 2023
Copy link
Owner

@JohnSundell JohnSundell left a comment

Choose a reason for hiding this comment

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

Looks great now, thanks a lot @harrydayexe! 😀

@JohnSundell JohnSundell merged commit 271926b into JohnSundell:master May 9, 2023
@harrydayexe harrydayexe deleted the add-time branch May 10, 2023 15:22
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.

3 participants