Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show the back end header on scroll-up #6569

Merged
merged 6 commits into from Nov 29, 2023

Conversation

leofeyer
Copy link
Member

@leofeyer leofeyer commented Nov 27, 2023

@leofeyer leofeyer added this to the 5.3 milestone Nov 27, 2023
@leofeyer leofeyer self-assigned this Nov 27, 2023
@leofeyer leofeyer requested a review from ausi November 27, 2023 15:25
@ausi
Copy link
Member

ausi commented Nov 27, 2023

Just for the record: I’m not a huge fan of these show-on-scroll-up fixed headers. I’d prefer keeping it fixed all the time.

@aschempp
Copy link
Member

aschempp commented Nov 27, 2023

see leofeyer#9 for a more universal Stimulus solution. The advantages are

  1. it can be applied to any element, not just the <header>.
  2. it would correctly handle the <header> element being replaced on the page (e.g. through a Turbo frame request)
  3. It doesn't fail if there is another <header> on the page (e.g. inside a <section> of some content)
  4. it can be reused to add a scroll-up class, e.g. for a "back to top link" later on
  5. it would also be easy to add a user permission later on. Just skip the data attributes in the controller to disable it (contrary to a globally loaded script).

@leofeyer
Copy link
Member Author

Just for the record: I’m not a huge fan of these show-on-scroll-up fixed headers. I’d prefer keeping it fixed all the time.

I know. But in the survey, only 14 people voted for a sticky header and 42 people voted against it. Therefore, the default behavior should be a non-sticky header.

@leofeyer leofeyer requested a review from ausi November 28, 2023 11:25
Copy link
Member

@aschempp aschempp left a comment

Choose a reason for hiding this comment

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

This would make sure the is always a threshold value, even if the data attribute is not set. We could also set the default to 40 and omit it in our template if that's a sane default.

@leofeyer leofeyer merged commit 85fb139 into contao:5.x Nov 29, 2023
16 checks passed
@leofeyer leofeyer deleted the feature/show-on-scroll branch November 29, 2023 12:46
leofeyer added a commit to leofeyer/contao that referenced this pull request Dec 1, 2023
leofeyer added a commit that referenced this pull request Dec 2, 2023
Description
-----------

Implements #5385 

The idea is to add a tab menu that allows you to jump to a section of the current palette:

<img width="1262" alt="" src="https://github.com/contao/contao/assets/1192057/e761b75f-7c00-4f73-a5c1-dd0a5962f967">

I‘m not yet happy with the design, especially if the version menu is visible as well:

<img width="1262" alt="" src="https://github.com/contao/contao/assets/1192057/f3ecd6b4-b135-4943-9824-d3425c7c2dc4">

It can also get pretty big if there are many sections:

<img width="1260" alt="" src="https://github.com/contao/contao/assets/1192057/d6e79b0c-ded0-4e64-85db-a984c576029a">

It is even worse on mobile devices:

<img width="295" alt="" src="https://github.com/contao/contao/assets/1192057/2820076e-b9d5-46c8-80b4-cb8965e719ef">

A possible solution would be to apply `overflow-x: scroll` to the parent div, so you can swipe left and right if the list exceeds the available width. We don‘t use this kind of UI anywhere else, though.

<img width="1263" alt="" src="https://github.com/contao/contao/assets/1192057/c790544f-6fca-42a2-aa87-ac154c08c3f9">

What do you think?

Commits
-------

21dff58 Revert 'Show the back end header on scroll-up (see #6569)'
0f7a249 Add a tab menu to jump to palette sections
1041949 Add a min-height to the jump targets bar
2c7aa37 Adjust the scroll offset
079f789 Apply suggestions from code review
ea1decb Rebuild the assets
fritzmg added a commit to fritzmg/contao that referenced this pull request Jan 11, 2024
fritzmg added a commit to fritzmg/contao that referenced this pull request Jan 11, 2024
leofeyer added a commit that referenced this pull request Jan 12, 2024
Description
-----------

Follow up to #6390. This implements the `PageRedirect` page type as a page controller and thus deprecates the legacy `PageRedirect` class.

Commits
-------

e8cec55 implement redirect page as controller
595fb02 move deprecation to constructor
e69c79b update tests and deprecations
b2b7387 update the tests
b468da9 cs fix
e287426 cs fix
7bda9b2 update PageRegistry
453fd50 use base path
0c97022 disable content composition and remove pty
e468bd9 Update and run the tools (see #6398)
d804025 Update and run the tools
faf4598 cs fix
ecbf45d merge with 5.x
9b446dd fix merge conflicts
83001c6 Merge remote-tracking branch 'origin/5.x' into redirect-page-controller
9f5f476 use UrlUtil::makeAbsolutePath from #6569
fa9a24a re-add TL_PTY entry
8fed920 Revert "use UrlUtil::makeAbsolutePath from #6569"
d365b6b remove test
1412912 re-add test
d015f64 remove test
6563b3e whoopsie

Co-authored-by: leofeyer <1192057+leofeyer@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants