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

Fix #3233 -- Simple theme classless semantic HTML #3234

Merged
merged 1 commit into from Nov 12, 2023
Merged

Fix #3233 -- Simple theme classless semantic HTML #3234

merged 1 commit into from Nov 12, 2023

Conversation

pauloxnet
Copy link
Contributor

@pauloxnet pauloxnet commented Oct 30, 2023 •
edited

Pull Request Checklist

Resolves: #3233

  • Ensured tests pass and (if applicable) updated functional test output
  • Conformed to code style guidelines by running appropriate linting tools
  • Added tests for changed code
  • Updated documentation for changed code

@pauloxnet pauloxnet mentioned this pull request Oct 30, 2023
Closed
7 tasks
@pauloxnet
Copy link
Contributor Author

I'll wait to receive a fe feedback before completing this PR with updated tests

@pauloxnet pauloxnet force-pushed the feature/3233-simple-classless-semantic-html branch 3 times, most recently from c0e2d4d to a2ab81d Compare October 31, 2023 08:15
@justinmayer justinmayer requested review from avaris and a team November 10, 2023 20:06
avaris
avaris requested changes Nov 10, 2023
View reviewed changes
Copy link
Member

@avaris avaris left a comment •
edited

Choose a reason for hiding this comment

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

Left some minor suggestions to docs. Overall looks good to me. Obviously, needs updates to the test output.

docs/settings.rst Outdated Show resolved Hide resolved
docs/settings.rst Outdated Show resolved Hide resolved
@pauloxnet pauloxnet force-pushed the feature/3233-simple-classless-semantic-html branch 2 times, most recently from b91cfda to 4a06da9 Compare November 11, 2023 10:12
@pauloxnet
Copy link
Contributor Author

Left some minor suggestions to docs.

Thanks, I've applied both of your suggestions to my code.

Overall looks good to me. Obviously, needs updates to the test output.

I was waiting for some feedback before fixing tests, thank to your review I'll do it.

@justinmayer
Copy link
Member

@pauloxnet: Wonderful. We are getting close to releasing Pelican 4.9 and would like to include this PR, so thank you for being so responsive! 🌟

@pauloxnet
Copy link
Contributor Author

@pauloxnet: Wonderful. We are getting close to releasing Pelican 4.9 and would like to include this PR, so thank you for being so responsive! 🌟

I would also like to see this improved version of the "simple" theme in Pelican 4.9. ✨

I'll try to find time to work on it this weekend. 🤞🏻

@pauloxnet pauloxnet force-pushed the feature/3233-simple-classless-semantic-html branch from 4a06da9 to c0c415c Compare November 11, 2023 12:51
@pauloxnet pauloxnet force-pushed the feature/3233-simple-classless-semantic-html branch from c0c415c to 6059675 Compare November 11, 2023 13:10
@pauloxnet
Copy link
Contributor Author

@avaris tests are fixed, there's still a requested change from you I think. ;-)

@justinmayer I think the PR is ready to be merged :-)

avaris
avaris approved these changes Nov 11, 2023
View reviewed changes
Copy link
Member

@avaris avaris left a comment

Choose a reason for hiding this comment

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

Thanks!

justinmayer
justinmayer approved these changes Nov 12, 2023
View reviewed changes
Copy link
Member

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

Many thanks to @pauloxnet for the enhancement and to @avaris for reviewing. 👍

@justinmayer justinmayer merged commit be5afa3 into getpelican:master Nov 12, 2023
15 checks passed
@pauloxnet pauloxnet deleted the feature/3233-simple-classless-semantic-html branch November 12, 2023 13:32
vapier
vapier reviewed Jan 29, 2024
View reviewed changes
pelican/themes/simple/templates/pagination.html
<a href="{{ SITEURL }}/{{ articles_next_page.url }}">&raquo;</a>
<a href="{{ SITEURL }}/{{ last_page.url }}">&#8649;</a>
<li><a href="{{ SITEURL }}/{{ articles_next_page.url }}">&rang;</a></li>
<li><a href="{{ SITEURL }}/{{ last_page.url }}">&Rang;</a></li>
Copy link

Choose a reason for hiding this comment

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

was it really intentional to move these into dedicated bullet points ? prior to 4.9, these would be on one-line, but now it's split out.

prior-to 4.9

Page 1 / 2 » ⇉

after this commit

* Page 1 / 2
* ⟩
* ⟫

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vapier vapier vapier left review comments

@avaris avaris avaris approved these changes

@justinmayer justinmayer justinmayer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use classless semantic HTML in the "simple" theme
4 participants
@pauloxnet @justinmayer @vapier @avaris