Skip to content

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens - #38148

Merged
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix
Oct 11, 2023
Merged

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens#38148
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix

Conversation

@AlenkaF

@AlenkaFAlenkaF commented Oct 9, 2023

Copy link
Copy Markdown
Member

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 #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.


/* Change header hight to make the logo a bit larger */
/* only on wider screens */
@media only screen and (min-width: 950px){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this will help any as the version switcher and search buttons are already causing issues (see pydata/pydata-sphinx-theme#1493 (comment)), but am trying locally now to see.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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:

Screenshot 2023-10-09 at 11 08 31

  • just over 1380px and bigger header height:

Screenshot 2023-10-09 at 11 08 23

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

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

@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 in the smaller header case as the current one is not readable on smaller screens.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 11, 2023
@AlenkaF
AlenkaF marked this pull request as ready for review October 11, 2023 10:16
@jorisvandenbossche

Copy link
Copy Markdown
Member

I would still change the logo to Chevron-only 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)

@jorisvandenbosschejorisvandenbossche changed the title GH-37947:[Docs] Keep header size default (small) on smaller screensGH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

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.

@jorisvandenbosschejorisvandenbossche changed the title GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensGH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@jorisvandenbossche
jorisvandenbossche merged commit 1cc0f14 into apache:mainOct 11, 2023
@jorisvandenbosschejorisvandenbossche removed the awaiting change review Awaiting change review label Oct 11, 2023
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #38209has been automatically assigned in GitHub to PR creator.

raulcd pushed a commit that referenced this pull request Oct 12, 2023
…default (small) on smaller screens (#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 #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.
* Closes: #38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
AlenkaF deleted the gh-37947-new-theme-bug-fix branch December 13, 2023 14:12
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for freeto 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.

[Docs] Reduce width of header items and keep header height default (small) on smaller screens

2 participants

@AlenkaF@jorisvandenbossche
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens by AlenkaF · Pull Request #38148 · apache/arrow · GitHub
Skip to content

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens - #38148

Merged
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix
Oct 11, 2023
Merged

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens#38148
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix

Conversation

@AlenkaF

@AlenkaFAlenkaF commented Oct 9, 2023

Copy link
Copy Markdown
Member

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 #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.


/* Change header hight to make the logo a bit larger */
/* only on wider screens */
@media only screen and (min-width: 950px){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this will help any as the version switcher and search buttons are already causing issues (see pydata/pydata-sphinx-theme#1493 (comment)), but am trying locally now to see.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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:

Screenshot 2023-10-09 at 11 08 31

  • just over 1380px and bigger header height:

Screenshot 2023-10-09 at 11 08 23

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

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

@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 in the smaller header case as the current one is not readable on smaller screens.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 11, 2023
@AlenkaF
AlenkaF marked this pull request as ready for review October 11, 2023 10:16
@jorisvandenbossche

Copy link
Copy Markdown
Member

I would still change the logo to Chevron-only 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)

@jorisvandenbosschejorisvandenbossche changed the title GH-37947:[Docs] Keep header size default (small) on smaller screensGH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

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.

@jorisvandenbosschejorisvandenbossche changed the title GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensGH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@jorisvandenbossche
jorisvandenbossche merged commit 1cc0f14 into apache:mainOct 11, 2023
@jorisvandenbosschejorisvandenbossche removed the awaiting change review Awaiting change review label Oct 11, 2023
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #38209has been automatically assigned in GitHub to PR creator.

raulcd pushed a commit that referenced this pull request Oct 12, 2023
…default (small) on smaller screens (#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 #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.
* Closes: #38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
AlenkaF deleted the gh-37947-new-theme-bug-fix branch December 13, 2023 14:12
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for freeto 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.

[Docs] Reduce width of header items and keep header height default (small) on smaller screens

2 participants

@AlenkaF@jorisvandenbossche
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens by AlenkaF · Pull Request #38148 · apache/arrow · GitHub
Skip to content

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens - #38148

Merged
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix
Oct 11, 2023
Merged

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens#38148
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix

Conversation

@AlenkaF

@AlenkaFAlenkaF commented Oct 9, 2023

Copy link
Copy Markdown
Member

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 #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.


/* Change header hight to make the logo a bit larger */
/* only on wider screens */
@media only screen and (min-width: 950px){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this will help any as the version switcher and search buttons are already causing issues (see pydata/pydata-sphinx-theme#1493 (comment)), but am trying locally now to see.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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:

Screenshot 2023-10-09 at 11 08 31

  • just over 1380px and bigger header height:

Screenshot 2023-10-09 at 11 08 23

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

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

@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 in the smaller header case as the current one is not readable on smaller screens.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 11, 2023
@AlenkaF
AlenkaF marked this pull request as ready for review October 11, 2023 10:16
@jorisvandenbossche

Copy link
Copy Markdown
Member

I would still change the logo to Chevron-only 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)

@jorisvandenbosschejorisvandenbossche changed the title GH-37947:[Docs] Keep header size default (small) on smaller screensGH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

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.

@jorisvandenbosschejorisvandenbossche changed the title GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensGH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@jorisvandenbossche
jorisvandenbossche merged commit 1cc0f14 into apache:mainOct 11, 2023
@jorisvandenbosschejorisvandenbossche removed the awaiting change review Awaiting change review label Oct 11, 2023
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #38209has been automatically assigned in GitHub to PR creator.

raulcd pushed a commit that referenced this pull request Oct 12, 2023
…default (small) on smaller screens (#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 #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.
* Closes: #38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
AlenkaF deleted the gh-37947-new-theme-bug-fix branch December 13, 2023 14:12
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for freeto 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.

[Docs] Reduce width of header items and keep header height default (small) on smaller screens

2 participants

@AlenkaF@jorisvandenbossche
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens by AlenkaF · Pull Request #38148 · apache/arrow · GitHub
Skip to content

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens - #38148

Merged
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix
Oct 11, 2023
Merged

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens#38148
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix

Conversation

@AlenkaF

@AlenkaFAlenkaF commented Oct 9, 2023

Copy link
Copy Markdown
Member

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 #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.


/* Change header hight to make the logo a bit larger */
/* only on wider screens */
@media only screen and (min-width: 950px){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this will help any as the version switcher and search buttons are already causing issues (see pydata/pydata-sphinx-theme#1493 (comment)), but am trying locally now to see.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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:

Screenshot 2023-10-09 at 11 08 31

  • just over 1380px and bigger header height:

Screenshot 2023-10-09 at 11 08 23

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

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

@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 in the smaller header case as the current one is not readable on smaller screens.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 11, 2023
@AlenkaF
AlenkaF marked this pull request as ready for review October 11, 2023 10:16
@jorisvandenbossche

Copy link
Copy Markdown
Member

I would still change the logo to Chevron-only 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)

@jorisvandenbosschejorisvandenbossche changed the title GH-37947:[Docs] Keep header size default (small) on smaller screensGH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

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.

@jorisvandenbosschejorisvandenbossche changed the title GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensGH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@jorisvandenbossche
jorisvandenbossche merged commit 1cc0f14 into apache:mainOct 11, 2023
@jorisvandenbosschejorisvandenbossche removed the awaiting change review Awaiting change review label Oct 11, 2023
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #38209has been automatically assigned in GitHub to PR creator.

raulcd pushed a commit that referenced this pull request Oct 12, 2023
…default (small) on smaller screens (#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 #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.
* Closes: #38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
AlenkaF deleted the gh-37947-new-theme-bug-fix branch December 13, 2023 14:12
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for freeto 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.

[Docs] Reduce width of header items and keep header height default (small) on smaller screens

2 participants

@AlenkaF@jorisvandenbossche
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens by AlenkaF · Pull Request #38148 · apache/arrow · GitHub
Skip to content

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens - #38148

Merged
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix
Oct 11, 2023
Merged

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens#38148
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix

Conversation

@AlenkaF

@AlenkaFAlenkaF commented Oct 9, 2023

Copy link
Copy Markdown
Member

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 #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.


/* Change header hight to make the logo a bit larger */
/* only on wider screens */
@media only screen and (min-width: 950px){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this will help any as the version switcher and search buttons are already causing issues (see pydata/pydata-sphinx-theme#1493 (comment)), but am trying locally now to see.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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:

Screenshot 2023-10-09 at 11 08 31

  • just over 1380px and bigger header height:

Screenshot 2023-10-09 at 11 08 23

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

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

@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 in the smaller header case as the current one is not readable on smaller screens.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 11, 2023
@AlenkaF
AlenkaF marked this pull request as ready for review October 11, 2023 10:16
@jorisvandenbossche

Copy link
Copy Markdown
Member

I would still change the logo to Chevron-only 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)

@jorisvandenbosschejorisvandenbossche changed the title GH-37947:[Docs] Keep header size default (small) on smaller screensGH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

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.

@jorisvandenbosschejorisvandenbossche changed the title GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensGH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@jorisvandenbossche
jorisvandenbossche merged commit 1cc0f14 into apache:mainOct 11, 2023
@jorisvandenbosschejorisvandenbossche removed the awaiting change review Awaiting change review label Oct 11, 2023
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #38209has been automatically assigned in GitHub to PR creator.

raulcd pushed a commit that referenced this pull request Oct 12, 2023
…default (small) on smaller screens (#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 #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.
* Closes: #38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
AlenkaF deleted the gh-37947-new-theme-bug-fix branch December 13, 2023 14:12
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for freeto 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.

[Docs] Reduce width of header items and keep header height default (small) on smaller screens

2 participants

@AlenkaF@jorisvandenbossche
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens by AlenkaF · Pull Request #38148 · apache/arrow · GitHub
Skip to content

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens - #38148

Merged
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix
Oct 11, 2023
Merged

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens#38148
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix

Conversation

@AlenkaF

@AlenkaFAlenkaF commented Oct 9, 2023

Copy link
Copy Markdown
Member

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 #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.


/* Change header hight to make the logo a bit larger */
/* only on wider screens */
@media only screen and (min-width: 950px){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this will help any as the version switcher and search buttons are already causing issues (see pydata/pydata-sphinx-theme#1493 (comment)), but am trying locally now to see.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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:

Screenshot 2023-10-09 at 11 08 31

  • just over 1380px and bigger header height:

Screenshot 2023-10-09 at 11 08 23

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

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

@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 in the smaller header case as the current one is not readable on smaller screens.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 11, 2023
@AlenkaF
AlenkaF marked this pull request as ready for review October 11, 2023 10:16
@jorisvandenbossche

Copy link
Copy Markdown
Member

I would still change the logo to Chevron-only 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)

@jorisvandenbosschejorisvandenbossche changed the title GH-37947:[Docs] Keep header size default (small) on smaller screensGH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

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.

@jorisvandenbosschejorisvandenbossche changed the title GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensGH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@jorisvandenbossche
jorisvandenbossche merged commit 1cc0f14 into apache:mainOct 11, 2023
@jorisvandenbosschejorisvandenbossche removed the awaiting change review Awaiting change review label Oct 11, 2023
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #38209has been automatically assigned in GitHub to PR creator.

raulcd pushed a commit that referenced this pull request Oct 12, 2023
…default (small) on smaller screens (#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 #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.
* Closes: #38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
AlenkaF deleted the gh-37947-new-theme-bug-fix branch December 13, 2023 14:12
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for freeto 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.

[Docs] Reduce width of header items and keep header height default (small) on smaller screens

2 participants

@AlenkaF@jorisvandenbossche
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens by AlenkaF · Pull Request #38148 · apache/arrow · GitHub
Skip to content

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens - #38148

Merged
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix
Oct 11, 2023
Merged

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens#38148
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix

Conversation

@AlenkaF

@AlenkaFAlenkaF commented Oct 9, 2023

Copy link
Copy Markdown
Member

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 #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.


/* Change header hight to make the logo a bit larger */
/* only on wider screens */
@media only screen and (min-width: 950px){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this will help any as the version switcher and search buttons are already causing issues (see pydata/pydata-sphinx-theme#1493 (comment)), but am trying locally now to see.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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:

Screenshot 2023-10-09 at 11 08 31

  • just over 1380px and bigger header height:

Screenshot 2023-10-09 at 11 08 23

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

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

@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 in the smaller header case as the current one is not readable on smaller screens.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 11, 2023
@AlenkaF
AlenkaF marked this pull request as ready for review October 11, 2023 10:16
@jorisvandenbossche

Copy link
Copy Markdown
Member

I would still change the logo to Chevron-only 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)

@jorisvandenbosschejorisvandenbossche changed the title GH-37947:[Docs] Keep header size default (small) on smaller screensGH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

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.

@jorisvandenbosschejorisvandenbossche changed the title GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensGH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@jorisvandenbossche
jorisvandenbossche merged commit 1cc0f14 into apache:mainOct 11, 2023
@jorisvandenbosschejorisvandenbossche removed the awaiting change review Awaiting change review label Oct 11, 2023
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #38209has been automatically assigned in GitHub to PR creator.

raulcd pushed a commit that referenced this pull request Oct 12, 2023
…default (small) on smaller screens (#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 #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.
* Closes: #38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
AlenkaF deleted the gh-37947-new-theme-bug-fix branch December 13, 2023 14:12
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for freeto 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.

[Docs] Reduce width of header items and keep header height default (small) on smaller screens

2 participants

@AlenkaF@jorisvandenbossche
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens by AlenkaF · Pull Request #38148 · apache/arrow · GitHub
Skip to content

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens - #38148

Merged
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix
Oct 11, 2023
Merged

GH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screens#38148
jorisvandenbossche merged 2 commits into
apache:mainfrom
AlenkaF:gh-37947-new-theme-bug-fix

Conversation

@AlenkaF

@AlenkaFAlenkaF commented Oct 9, 2023

Copy link
Copy Markdown
Member

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 #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.


/* Change header hight to make the logo a bit larger */
/* only on wider screens */
@media only screen and (min-width: 950px){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this will help any as the version switcher and search buttons are already causing issues (see pydata/pydata-sphinx-theme#1493 (comment)), but am trying locally now to see.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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:

Screenshot 2023-10-09 at 11 08 31

  • just over 1380px and bigger header height:

Screenshot 2023-10-09 at 11 08 23

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

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 9, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

@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 in the smaller header case as the current one is not readable on smaller screens.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 11, 2023
@AlenkaF
AlenkaF marked this pull request as ready for review October 11, 2023 10:16
@jorisvandenbossche

Copy link
Copy Markdown
Member

I would still change the logo to Chevron-only 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)

@jorisvandenbosschejorisvandenbossche changed the title GH-37947:[Docs] Keep header size default (small) on smaller screensGH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@AlenkaF

Copy link
Copy Markdown
MemberAuthor

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.

@jorisvandenbosschejorisvandenbossche changed the title GH-37947: [Docs] Reduce width of header items and keep header height default (small) on smaller screensGH-38209: [Docs] Reduce width of header items and keep header height default (small) on smaller screensOct 11, 2023
@jorisvandenbossche
jorisvandenbossche merged commit 1cc0f14 into apache:mainOct 11, 2023
@jorisvandenbosschejorisvandenbossche removed the awaiting change review Awaiting change review label Oct 11, 2023
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #38209has been automatically assigned in GitHub to PR creator.

raulcd pushed a commit that referenced this pull request Oct 12, 2023
…default (small) on smaller screens (#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 #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.
* Closes: #38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1cc0f14.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
AlenkaF deleted the gh-37947-new-theme-bug-fix branch December 13, 2023 14:12
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…eight default (small) on smaller screens (apache#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 apache#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.
* Closes: apache#38209
Authored-by: AlenkaF <frim.alenka@gmail.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for freeto 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.

[Docs] Reduce width of header items and keep header height default (small) on smaller screens

2 participants

@AlenkaF@jorisvandenbossche