Skip to content

Conversation

@pmattmann
Copy link
Member

related to #5608

Not a auto-scroll
But a link to scroll to 'today'

To test:

  • create a camp with a start-date in the past and an end-date in the future.
  • goto dashboard
  • see icon on the top right

@pmattmann pmattmann requested a review from a team October 13, 2024 18:56
@manuelmeister manuelmeister added the deploy! Creates a feature branch deployment for this PR label Oct 13, 2024
@github-actions
Copy link

github-actions bot commented Oct 13, 2024

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

Copy link
Member

@manuelmeister manuelmeister left a comment

Choose a reason for hiding this comment

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

Very cool idea!

I think clock-in-custom could be more clear. Also I'm not sure of the placement of this button. Maybe it could be a floating action button that is only visible if the tbody isn't visible (IntersectionObserver).

:aria-labelledby="dayUri + 'th'"
>
<tr class="day-header__row">
<tr :ref="days[dayUri].id" class="day-header__row">
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be set on the tbody and preferably as an id. Which we could reflect in the url.

Then the tbody should have a scroll-margin-top with the same value as the top of the .day-header: So the navbar does not cover the content:

Bildschirmfoto 2024-10-14 um 08 38 49

If solved as an id, someone could then share this link and it will automatically go to the same day.

Copy link
Member

@usu usu left a comment

Choose a reason for hiding this comment

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

Agree to comment of @manuelmeister.
As this button/icon is really only visible during the actual camp, I would be ok with an action button with actual text (for example: "go to today" or shorter "today")

Also for me the scrolling action is always a bit too far (day header is not shown anymore), both on mobile and on desktop. I think this could be fine-tuned either with the block option or with scroll-margin-top css.

Code looks good to me.

@pmattmann pmattmann force-pushed the feature/dashboard-scroll-today branch from b1024c2 to 8059636 Compare November 3, 2024 16:05
@pmattmann
Copy link
Member Author

Agree to comment of @manuelmeister. As this button/icon is really only visible during the actual camp, I would be ok with an action button with actual text (for example: "go to today" or shorter "today")

  • Done: Button has text 'today'

Also for me the scrolling action is always a bit too far (day header is not shown anymore), both on mobile and on desktop. I think this could be fine-tuned either with the block option or with scroll-margin-top css.

  • Scroll-Issue is fixed - code contains magic numbers :/
    Better solution would be welcome.

@pmattmann pmattmann requested a review from a team November 3, 2024 16:43
Copy link
Member

@manuelmeister manuelmeister left a comment

Choose a reason for hiding this comment

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

Nice changes. I think I will eventually create an improved hover button solution. But this change already improves the UX.

@manuelmeister manuelmeister added this pull request to the merge queue Nov 5, 2024
Merged via the queue into ecamp:devel with commit 1e67c90 Nov 5, 2024
30 checks passed
@pmattmann pmattmann deleted the feature/dashboard-scroll-today branch November 25, 2024 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy! Creates a feature branch deployment for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants