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

“Edit this page” → “Edit on GitHub/GitLab/Bitbucket” #1177

Merged
merged 3 commits into from
Feb 14, 2023
Merged

“Edit this page” → “Edit on GitHub/GitLab/Bitbucket” #1177

merged 3 commits into from
Feb 14, 2023

Conversation

jeanas
Copy link
Contributor

@jeanas jeanas commented Feb 13, 2023

Fixes #1172

Open question: should the “edit-this-page.html” template be renamed?

TODO: add tests (I need to figure out how to write them; help welcome).

Copy link
Collaborator

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

this seems generally pretty good to me!

For your tests question, I think that you might only need to change the regression tests that are here:

https://github.com/pydata/pydata-sphinx-theme/tree/main/tests/test_build

I suspect that when you run the test suite you'll see errors in the regression tests for some HTML templates in that folder. If you delete the files and re-run, then they will be updated with the changes that are reflected from your PR.

@jeanas
Copy link
Contributor Author

jeanas commented Feb 14, 2023

Thanks for the advice! I have updated the tests, now they also test for the link text.

Copy link
Collaborator

@12rambau 12rambau 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 taking the time to make a PR. For the sake of real estate consistency, can we go one step further and use an icon instead of the provider name? exactly as they are doing in RDT

@choldgraf
Copy link
Collaborator

I'm not sure why some jobs were failing, I think there might have been an intermittent error?

My 2 cents on the icon question: I am +1 to icons, though don't feel it is necessary to block this PR on having icons. So I'd suggest that we ask @jeanas if they think this would be simple for them to implement, and if not then we try to merge this with the words and then leave icons to a future improvement / issue.

(if we do implement icons, note that we'll need to include something like an aria-label so that GitHub is written down somewhere)

@jeanas
Copy link
Contributor Author

jeanas commented Feb 14, 2023

@12rambau I’m not sure what you mean exactly for the icons, could you clarify it for me? On RTD, they are displaying “[GitHub icon] Edit on GitHub”, but from the wording of your comment, I have the impression that you mean “Edit on [GitHub icon]”.

Personally, I like the current “[Pencil icon] Edit on GitHub” because there is already a GitHub icon on the top right (for going to the project repository) if you have enabled it.

@drammock
Copy link
Collaborator

Personally, I like the current “[Pencil icon] Edit on GitHub” because there is already a GitHub icon on the top right (for going to the project repository) if you have enabled it.

Agreed.

Re: the failing CIs:

There is a linter error, which might mean that your pre-commit hooks aren't set up correctly. See here: https://pydata-sphinx-theme.readthedocs.io/en/latest/community/setup.html#setup-pre-commit The other failures look at first glance like network problems, hopefully transient. Let's see if they fail again on next push.

@12rambau
Copy link
Collaborator

you convinced me, let's stick to the pencil icon only ;-)

@jeanas
Copy link
Contributor Author

jeanas commented Feb 14, 2023

Tests should be better now. Also, flake8 rightfully reported a typo <blush>... I've fixed it and amended the test so that it would have caught it.

@drammock drammock merged commit 8d8bcb8 into pydata:main Feb 14, 2023
@drammock
Copy link
Collaborator

thx @jeanas !

@jeanas jeanas deleted the edit-this-page branch February 14, 2023 18:31
@jeanas
Copy link
Contributor Author

jeanas commented Feb 14, 2023

Thank you all for your review time!

12rambau added a commit to 12rambau/pydata-sphinx-theme that referenced this pull request Mar 2, 2023
* Fix extra whitespace in sidebars (pydata#1115)

* Fix extra whitespace in sidebars

* Searchbox

* Update src/pydata_sphinx_theme/__init__.py

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* make test pass

* Fix template filter to remove empty files

* ABlog in template test

* Move clear search button to primary sidebar

* Move search clear button to top of article

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* FIX: Use logo_url instead deprecated logo in theme (pydata#1094) (pydata#1097)

resolves pydata#1094

* ENH/MAINT: avoid overwriting the HtmlTranslator (pydata#1105)

Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Fix pydata#143
Fix pydata#94

* fix: align sidebar sliding with the buttons (pydata#1123)

* fix: aline the sidebar sliding with the buttons

* build: force test to run on all platform
if one platform is failing we cannot see if it's platform related as the other were closed

* fix: use correct path for documentation logo

* MAINT: Improve font sizing (pydata#1129)

Fix pydata#1001

* MAINT: Refactor workflows to reduce test dependencies (pydata#1136)

* MAINT: update prerelease workflow (pydata#1140)

* ABlog: Updates for new HTML structure (pydata#1118)

* ABlog: Updates for new HTML structure

* Update templates for latest release

* Bump to dev0

* Standardize logo image behavior between Sphinx and this theme (pydata#1132)

Co-authored-by: Daniel McCloy <dan@mccloy.info>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: Chris Holdgraf <choldgraf@berkeley.edu>

* 0.13.0rc1

* Build(deps): Bump http-cache-semantics from 4.1.0 to 4.1.1 (pydata#1154)

* DOC: Use only shield.io for badges in README (pydata#1152)

* Copyright semicolon (pydata#1160)

* FIX: Flex behavior should shrink header items instead of brand (pydata#1158)

Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Fixes pydata#1143

* STYLE: lint the documentation with Doc8 (pydata#1150)

Fix pydata#1139

* Add test for internationalization and translations (pydata#1138)

* FIX: Javascript incorrect check for variable (pydata#1166)

* MAINT: update pypi classifiers (pydata#1153)

Fix pydata#1106

* remove emoji from landing page (pydata#1151)

* add fa icons instead of emoji

* remove fix for emojis

* use markup for readme emojis

* use pst-color-primary instead of sd-text-primary

* make our semantic colors available as classes

* try again

---------

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* FIX: Narrow scope of style rule for GitHub & GitLab link shortening (pydata#1167)

Fixes pydata#1156

* ENH: Add breadcrumbs to article header (pydata#1142)

* ENH: Add breadcrumbs to article header

* Update src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/breadcrumbs.html

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* More fixes

* Improving nested page behavior

* Documenting breadcrumbs

* Update src/pydata_sphinx_theme/assets/styles/components/_breadcrumbs.scss

Co-authored-by: Rambaud Pierrick <12rambau@users.noreply.github.com>

* Breacrumbs have link color

---------

Co-authored-by: Tania Allard <taniar.allard@gmail.com>
Co-authored-by: Rambaud Pierrick <12rambau@users.noreply.github.com>

* Degrade gracefully when JavaScript is disabled (pydata#1146)

* Fix header vertical spacing and jupyter-sphinx cells (pydata#1164)

Fixes undefined

* RLS: v0.13.0rc2 (pydata#1170)

* DOCS: admonition customization (pydata#1155)

* first draft of the admonition customization

* typo in doc link

* flesh out admon. customization example; DRY

* use :code:rst instead of :literal:

* Update docs/_static/custom.css

---------

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* Fix article header CSS (pydata#1171)

* “Edit this page” → “Edit on GitHub/GitLab/Bitbucket” (pydata#1177)

* “Edit this page” → “Edit on GitHub/GitLab/Bitbucket”

Fixes pydata#1172

* Add tests

* Fix typo

* Properly handle default_mode=auto when writing logos (pydata#1183)

We used to only defaulting to the light version when `default_mode` was
undefined, not when it was explicitly set to `auto`. We also need to
handle the latter, as the new test shows.

Closes pydata#1180

Co-authored-by: Jérémy Bobbio (Lunar) <lunar@softwareheritage.org>

* fix: correctly add DOM listeners (pydata#1179)

fix adding DOM listeners

* maint: update GitLab URL tests (pydata#1186)

Co-authored-by: JoerivanEngelen <joerivanengelen@hotmail.com>

* Standardize template structure in more sections (pydata#1184)

* Standardize template structure in all sections

* Fixing footer behavior

* Update docs/user_guide/layout.rst

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* Remove use of id= as much as possible

---------

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* maint: remove sphinx-panels support; remove deprecated config shims (pydata#1188)

* Minor style improvements to ablog (pydata#1185)

* RLS: v0.13.0rc3

* dev0

* FIX: Some style bugs (pydata#1191)

* FIX: Some style bugs

* Move link word wrap to global rule

* DOCS: Add internationalization instructions (pydata#1178)

Co-authored-by: James McKinney <26463+jpmckinney@users.noreply.github.com>

* Refactor contributing docs to be more modular (pydata#1173)

* Dev0

* Fix github gitlab brand (pydata#1194)

* RLS: v0.13.0rc4

* FIX: Make wide equations scroll (pydata#1196)

* Fix math scrollbars for realz (pydata#1198)

* Fix math scrollbars for realz

* Update src/pydata_sphinx_theme/assets/styles/content/_math.scss

* Update src/pydata_sphinx_theme/assets/styles/content/_math.scss

* copy_logo_images: do not render dynamic Sphinx template content (pydata#1204)

* copy_logo_images: do not render dynamic Sphinx template content when copying logo image files

* Update src/pydata_sphinx_theme/__init__.py

---------

Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>

* Add conditional check for last-updated template (pydata#1201)

* Add conditional check for last-updated template

* Whitespace

* Properly set configuration with app.builder.theme_options (pydata#1199)

* Properly set configuration

* Dict to values

* Making it explicit in a function

* Better name

* Fix test

* Foot

* Revert complex config set

* Clarify docs

* Use CSS transform for skip link (pydata#1206)

* feat: Add full i18n support (pydata#1192)

Co-authored-by: Daniel McCloy <dan@mccloy.info>
Co-authored-by: James McKinney <26463+jpmckinney@users.noreply.github.com>
Co-authored-by: Chris Holdgraf <choldgraf@berkeley.edu>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>

* Dev0

* FIX: Remove icon links component when no icon links given (pydata#1209)

* RLS: 0.13.0rc5

* dev0

* FIX: Get theme options in a more robust way (pydata#1214)

* RLS: v0.13.0rc6

* Make heading-style use the font-weight-heading value (pydata#1213)

* Make heading-style use the font-weight-heading value

* Separate font-weight setting for content headers and admonitions

* Flip var to be consistent with --pst-font-weight-heading instead

* RLS: v0.13.0

* bump: dev0

* DOCS: Remove <p> from announcement sample text (pydata#1223)

---------

Co-authored-by: Chris Holdgraf <choldgraf@berkeley.edu>
Co-authored-by: Daniel McCloy <dan@mccloy.info>
Co-authored-by: Nico Albers <nico.albers@aboutyou.com>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Brendan Heberlein <bheberlein@wisc.edu>
Co-authored-by: Tania Allard <taniar.allard@gmail.com>
Co-authored-by: Lunar <lunar@debian.org>
Co-authored-by: Jean Abou-Samra <jean@abou-samra.fr>
Co-authored-by: Jérémy Bobbio (Lunar) <lunar@softwareheritage.org>
Co-authored-by: JoerivanEngelen <joerivanengelen@hotmail.com>
Co-authored-by: James McKinney <26463+jpmckinney@users.noreply.github.com>
Co-authored-by: James Addison <55152140+jayaddison@users.noreply.github.com>
Co-authored-by: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com>
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.

Consider amending “Edit this page” text
4 participants