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.

Sign up for GitHub

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

Jump to bottom

Do not limit header height #4073

Merged
merged 2 commits into from
Mar 4, 2022
Merged

Do not limit header height #4073

merged 2 commits into from
Mar 4, 2022

Conversation

tomaskikutis
Copy link
Member

SDESK-6364

@petrjasek petrjasek added this to the 2.4.1 milestone Mar 3, 2022
fritzSF
fritzSF reviewed Mar 3, 2022
View reviewed changes
Copy link
Contributor

@fritzSF fritzSF left a comment

Choose a reason for hiding this comment

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

Whitout limiting the header hight there will be no transition. I'm against killing general UX features because of requests of one customer or some edge cases. IMO this is generally not the right way to go.

fritzSF
fritzSF requested changes Mar 3, 2022
View reviewed changes
Copy link
Contributor

@fritzSF fritzSF left a comment

Choose a reason for hiding this comment

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

Limiting the height to a large value (e.g. 100vh, 2000px or similar) might be sufficient to solve the problem and still preserve some transitioning while opening the header.

@tomaskikutis
Copy link
Member Author

Correct, I did drop the animation. Existing animation only worked when expanding and didn't impact the UI much in my opinion so I dropped it. I've added a proper implementation now that computes stuff dynamically and doesn't depend on any hard-coded height.

@tomaskikutis
Copy link
Member Author

animate.header.mp4

@fritzSF
Copy link
Contributor

fritzSF commented Mar 3, 2022

Niice! Looks much better than before.

@tomaskikutis tomaskikutis merged commit 4b036dd into superdesk:develop Mar 4, 2022
MarkLark86 pushed a commit to MarkLark86/superdesk-client-core that referenced this pull request Mar 9, 2022
@tomaskikutis @MarkLark86
Do not limit header height (superdesk#4073)
0fa4415
@tomaskikutis tomaskikutis mentioned this pull request Apr 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrjasek petrjasek petrjasek approved these changes

@fritzSF fritzSF fritzSF approved these changes

@pavlovicnemanja pavlovicnemanja pavlovicnemanja approved these changes

Assignees
No one assigned
Labels
merged
Projects
None yet
Milestone
2.4.1
Development

Successfully merging this pull request may close these issues.
5 participants
@tomaskikutis @fritzSF @petrjasek @pavlovicnemanja @MarkLark86