Skip to content

New website style + docs sorting modifications #2924

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bilhox
Copy link
Contributor

@bilhox bilhox commented Jun 13, 2024

Hello,

This PR aims to change the website style after years of no modifications. I understand this is considered as a BIG UI change and there is a huge talk on the topic, so I want to make you understand the old theme isn't gone ! Indeed, the goal is to propose for the moment the new style only for static doc generation. Furthermore, the default style when generating docs is the old style, if you want to test the new website style, make sure to read docs/README.md, a small paragraph was added.

Any feedbacks would be considered as a great opportunity to progress on this idea ! I hope you like the new style.

@bilhox bilhox requested a review from a team as a code owner June 13, 2024 19:52
@@ -1,25 +1,2 @@
/* Auto generated file: with make_docs.py . Docs go in docs/reST/ref/ . */
#define DOC_JOYSTICK "Pygame module for interacting with joysticks, gamepads, and trackballs."
#define DOC_JOYSTICK_INIT "init() -> None\nInitialize the joystick module."
Copy link
Member

Choose a reason for hiding this comment

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

Some failed checks are complaining about this, they can't compile the joystick module without this defined DOCs. I have no idea of how it works but I think some file or sphynx itself is reading the docs files and generating this entries. I am not sure why it's not finding them, have you run py setup.py docs --fullgeneration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

py setup.py docs --fullgeneration is deprecated as stated in docs/README.md. If I try to run it, I have an error with setuptools when it comes to call setup at the end of setup.py.

Copy link
Member

Choose a reason for hiding this comment

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

py setup.py docs --fullgeneration is deprecated as stated in docs/README.md. If I try to run it, I have an error with setuptools when it comes to call setup at the end of setup.py.

Weird, it works fine for me. Either way those #define need to come back, if you don't know how perhaps you should ask in #contributing (I don't know how either)

@bilhox
Copy link
Contributor Author

bilhox commented Jun 16, 2024

After some reviews with Damus, we found that there is a problem a problem of opacity on links with a feature when you hover it, you get some kind of infos about it.
In the meantime, some parts of the documentation have been reworked as it doesn't follow the style of other docs pages.

@Starbuck5
Copy link
Member

In the meantime, some parts of the documentation have been reworked as it doesn't follow the style of other docs pages.

If you want to change content in the docs, do it in a separate PR. That doesn't belong in this PR.

Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

As Starbuck mentioned, content changes should be in a separate PR so that this one and those content changes are not both contingent on the other. I also have a few gripes here that I would like to see addressed (or explained if not addressable)

@@ -1,4 +1,4 @@
.. include:: common.txt
.. include:: ../../common.txt
Copy link
Member

Choose a reason for hiding this comment

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

I don't know that I like the relative pathing here. Is there no way around it?

Comment on lines +17 to +19
You may want to test the new website style, you just have to edit
``docs/reST/conf.py`` and change the value of ``html_theme`` to ``sphinx_book_theme``.
(Make sure before to install the theme with ``python -m pip install sphinx_book_theme``)
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this PR supposed to implement the new style? I'm confused by what's intended here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, thank you for the review !

After a big talk on discord, a lot mentionned the idea of making a progressive transition between the old style and the new style. So right now the ideal would be to keep it experimental, and then we make a new PR that will make it default or no.

:glob:
:hidden:

extra_infos/color_list.rst
Copy link
Member

Choose a reason for hiding this comment

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

extra_infos doesn't sound right to me, maybe it should just be extra_info?

@ankith26 ankith26 marked this pull request as draft July 8, 2024 14:17
@Starbuck5
Copy link
Member

I wanted to check out the style, so I switched the theme in conf.py and installed the additional package. But it fails to build.

PS C:\Users\charl\Desktop\pygame-ce> python setup.py docs --f
WARNING: This command is deprecated and will be removed in the future.
Please use the following replacement: `C:\Users\charl\AppData\Local\Programs\Python\Python39\python.exe buildconfig\make_docs.py full_generation`

Executing sphinx in subprocess with args: ['C:\\Users\\charl\\AppData\\Local\\Programs\\Python\\Python39\\python.exe', '-m', 'sphinx', '-b', 'html', '-d', 'docs\\generated\\doctrees', '-D', 'headers_dest=src_c\\doc', '-D', 'headers_mkdirs=0', 'docs\\reST', 'docs\\generated', '-E']
Running Sphinx v7.1.2
C:\Users\charl\AppData\Local\Programs\Python\Python39\lib\site-packages\sphinxcontrib\applehelp\__init__.py:24: RemovedInSphinx80Warning: The alias 'sphinx.util.SkipProgressMessage' is deprecated, use 'sphinx.util.display.SkipProgressMessage' instead. Check CHANGES for Sphinx API modifications.
  from sphinx.util import SkipProgressMessage, progress_message
C:\Users\charl\AppData\Local\Programs\Python\Python39\lib\site-packages\sphinxcontrib\applehelp\__init__.py:24: RemovedInSphinx80Warning: The alias 'sphinx.util.progress_message' is deprecated, use 'sphinx.http_date.epoch_to_rfc1123' instead. Check CHANGES for Sphinx API modifications.
  from sphinx.util import SkipProgressMessage, progress_message
building [mo]: targets for 0 po files that are out of date
writing output...
building [html]: targets for 98 source files that are out of date
updating environment: [new config] 98 added, 0 changed, 0 removed
reading sources... [100%] tutorials/ko/빨간블록 검은블록/개요
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... WARNING: unsupported theme option 'home_uri' given
done
copying assets... copying static files... done
copying extra files... done
done
writing output... [  1%] c_api
Extension error (pydata_sphinx_theme):
Handler <function update_and_remove_templates at 0x00000221BDA35670> for event 'html-page-context' threw an exception (exception: 'dict object' has no attribute 'image_relative')

---
For help with compilation see:
    https://github.com/pygame-community/pygame-ce/wiki/Compiling-on-Windows
To contribute to pygame-ce development see:
    https://github.com/pygame-community/pygame-ce
---

Failed to build documentation

Manipulate sound sample data.

:doc:`ref/sprite`
Higher level objects to represent game images.
Copy link
Contributor

Choose a reason for hiding this comment

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

Game images? Why not game objects

@Notenlish
Copy link
Contributor

image
I like this new docs theme. But IMO, using green based colors would be better.

image
Why 2 seperate colors(orange happens when hovering over a link). I think it'd be better if the docs used only blue or green for link colors.

The docs also need to use the new pygame-ce logo.

image
This color could be changed.

@Notenlish
Copy link
Contributor

image
Would it be possible to move the contents so it aligns with the search bar?
Also can we get rid of the line to the left of contents

@bilhox
Copy link
Contributor Author

bilhox commented Sep 8, 2024

The docs also need to use the new pygame-ce logo.

Yes I didn't update the branch yet.

@bilhox
Copy link
Contributor Author

bilhox commented Sep 8, 2024

Why 2 seperate colors(orange happens when hovering over a link). I think it'd be better if the docs used only blue or green for link colors.

For the moment, this version directly uses the theme installed, I did no modification to it. I think I'll be moving it to custom themes so we can do changes on the colors and everything. (maybe next week)

@Notenlish
Copy link
Contributor

Notenlish commented Sep 8, 2024

image Would it be possible to move the contents so it aligns with the search bar? Also can we get rid of the line to the left of contents

image
I was thinking about something like this
this could be done by just using position: relative; in the container and then setting position: fixed and top: [some value]; in the contents element.

@Notenlish
Copy link
Contributor

image

What if we made the background of the code example white, and then simply just added background shadow?

something like this:
image

@Notenlish
Copy link
Contributor

Notenlish commented Sep 8, 2024

image
this should be 2023 - 2024 i think, not sure though

@Notenlish
Copy link
Contributor

image
Colors for dark theme definitely need to be changed

@Notenlish
Copy link
Contributor

image
The color contrast is bad when hovering over the items

same thing happens in dark theme too:
image

@Notenlish
Copy link
Contributor

image
Why is there an element to purposefully add margin bottom to make the sidebar have a scrollbar, even if the sidebar contains few items.

weird choice by the theme makers

@Notenlish Notenlish mentioned this pull request Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants