You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "AlenkaF (via GitHub)" <gi...@apache.org> on 2023/10/09 08:27:30 UTC
[PR] GH-37947:[Docs] Keep header size default (small) on smaller screens [arrow]
AlenkaF opened a new pull request, #38148:
URL: https://github.com/apache/arrow/pull/38148
### Rationale for this change
The Sphinx theme we have been using (PyData Sphinx Theme) has been pinned to an older version for a while now and with the https://github.com/apache/arrow/pull/36591 we have updated the code and are now using version 0.14.0 for the dev docs.
This PR fixes bugs we have encountered after the PR updating the theme has been merged.
### What changes are included in this PR?
- Have default header size for smaller screens and keep it increased for bigger screens.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37947:[Docs] Keep header size default (small) on smaller screens [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #38148:
URL: https://github.com/apache/arrow/pull/38148#discussion_r1350024167
##########
docs/source/_static/theme_overrides.css:
##########
@@ -21,12 +21,18 @@
/* Customizing with theme CSS variables */
:root {
- /* Change header hight to make the logo a bit larger */
- --pst-header-height: 6rem;
/* Make headings more bold */
--pst-font-weight-heading: 600;
}
+/* Change header hight to make the logo a bit larger */
+/* only on wider screens */
+@media only screen and (min-width: 950px){
Review Comment:
How did you decide on the min width of 950? From doing a quick experiment with the "Responsive design mode" in Firefox, I already see it wraparound (causing a very large header) at a width of around 1380 px
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37947:[Docs] Keep header size default (small) on smaller screens [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #38148:
URL: https://github.com/apache/arrow/pull/38148#issuecomment-1757266788
@jorisvandenbossche I changed "Specifications and Protocol" to just "Specifications" and "Language implementations" to just "Implementations". Then the double header comes up at approx 1170px and that is when we default back to the smaller header size.
I would still change the logo to [Chevron-only](https://arrow.apache.org/img/arrow-logo_chevrons_black-txt_white-bg.png) in the smaller header case as the current one is not readable on smaller screens.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38148:
URL: https://github.com/apache/arrow/pull/38148#issuecomment-1763287572
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14bf5312cb54841c8f75a3b86f068f3d127.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/17711936222) has more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37947:[Docs] Keep header size default (small) on smaller screens [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #38148:
URL: https://github.com/apache/arrow/pull/38148#discussion_r1350035885
##########
docs/source/_static/theme_overrides.css:
##########
@@ -21,12 +21,18 @@
/* Customizing with theme CSS variables */
:root {
- /* Change header hight to make the logo a bit larger */
- --pst-header-height: 6rem;
/* Make headings more bold */
--pst-font-weight-heading: 600;
}
+/* Change header hight to make the logo a bit larger */
+/* only on wider screens */
+@media only screen and (min-width: 950px){
Review Comment:
I am not sure this will help any as the version switcher and search buttons are already causing issues (see https://github.com/pydata/pydata-sphinx-theme/issues/1493#issuecomment-1746819706), but am trying locally now to see.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38148:
URL: https://github.com/apache/arrow/pull/38148#issuecomment-1758051279
:warning: GitHub issue #38209 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37947:[Docs] Keep header size default (small) on smaller screens [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #38148:
URL: https://github.com/apache/arrow/pull/38148#discussion_r1350028097
##########
docs/source/_static/theme_overrides.css:
##########
@@ -21,12 +21,18 @@
/* Customizing with theme CSS variables */
:root {
- /* Change header hight to make the logo a bit larger */
- --pst-header-height: 6rem;
/* Make headings more bold */
--pst-font-weight-heading: 600;
}
+/* Change header hight to make the logo a bit larger */
+/* only on wider screens */
+@media only screen and (min-width: 950px){
Review Comment:
Or this is only for the "mobile" layout? (where there is no header at all anymore, except for the logo and expand buttons)
I indeed suggested on the issue to do this in this case, but, we can maybe also already do it when you are on a normal but small screen, i.e. for the cases where all the header navbar items are visible, but start to wrap around because there is not enough space, like:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37947:[Docs] Keep header size default (small) on smaller screens [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #38148:
URL: https://github.com/apache/arrow/pull/38148#issuecomment-1757356797
> I would still change the logo to [Chevron-only](https://arrow.apache.org/img/arrow-logo_chevrons_black-txt_white-bg.png) in the smaller header case as the current one is not readable on smaller screens.
It's only the "Apache" part that is not readable, right? I agree it doesn't look great with the Apache not being readable, but on the other hand I think it is nice you still see "Arrow" .. (but I suppose having a logo with just Arrow without Apache will violate the rules of how the name should be used)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche merged PR #38148:
URL: https://github.com/apache/arrow/pull/38148
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screens [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #38148:
URL: https://github.com/apache/arrow/pull/38148#issuecomment-1757384115
> It's only the "Apache" part that is not readable, right?
Yes, correct.
> I agree it doesn't look great with the Apache not being readable, but on the other hand I think it is nice you still see "Arrow" .. (but I suppose having a logo with just Arrow without Apache will violate the rules of how the name should be used)
I do not think it is violating, Apache is still there, it is just very small and I think in that case using only chevron without Apache Arrow would be better. But I understand, Arrow will then not be there, which is a bummer.
No rush to change though. We can decide for 15.0.0 and have the header change included in 14.0.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37947:[Docs] Keep header size default (small) on smaller screens [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #38148:
URL: https://github.com/apache/arrow/pull/38148#discussion_r1350048127
##########
docs/source/_static/theme_overrides.css:
##########
@@ -21,12 +21,18 @@
/* Customizing with theme CSS variables */
:root {
- /* Change header hight to make the logo a bit larger */
- --pst-header-height: 6rem;
/* Make headings more bold */
--pst-font-weight-heading: 600;
}
+/* Change header hight to make the logo a bit larger */
+/* only on wider screens */
+@media only screen and (min-width: 950px){
Review Comment:
Ok, setting the minimum width of the screen to have the bigger header height to 1380px gave me two layouts to compare as the limit for the double header is a bit higher:
- just under 1380px and smaller header height:
- just over 1380px and bigger header height:
I agree the smaller header size is better in this case also (double navbar), but the logo is too small to read it and that is not something I would keep.
I propose to have the bigger header height for 1400px but in the case of default header height I would use a different logo: [Chevron-only logo](https://arrow.apache.org/img/arrow-logo_chevrons_black-txt_white-bg.png)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org