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

Wizard: Ensure wizard footer remains visible in repo step #2527

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rissh
Copy link

@rissh rissh commented Oct 22, 2024

  • Improved the layout of the repository step in the wizard to prevent the "Next" button from being hidden behind a scroll.
  • Adjusted the wizard size to ensure a better user experience without needing to scroll in the repository selection step.
  • Ensured consistent button positioning across varying screen resolutions.
  • Attached is a video demo showcasing how this change resolves it.
  • This is my approach to solving the issue, and I'm happy to consider alternative suggestions if anyone has a different idea.
Screencast.from.2024-10-22.11-59-24.webm

@app-sre-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@rissh
Copy link
Author

rissh commented Oct 22, 2024

Fixes #2467

@rissh rissh changed the title Fix: Ensure wizard footer remains visible in repo step Wizard: Ensure wizard footer remains visible in repo step Oct 22, 2024
Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.63%. Comparing base (a2b8883) to head (0598fb8).
Report is 123 commits behind head on main.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2527   +/-   ##
=======================================
  Coverage   83.63%   83.63%           
=======================================
  Files         162      162           
  Lines       17638    17638           
  Branches     1796     1796           
=======================================
  Hits        14751    14751           
  Misses       2865     2865           
  Partials       22       22           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2b8883...0598fb8. Read the comment docs.

@ezr-ondrej
Copy link
Collaborator

/ok-to-test

@ezr-ondrej
Copy link
Collaborator

/retest

@ezr-ondrej
Copy link
Collaborator

Hi! This now is in fixed height, but it always scrolls for me even on zoomed out screen where there is a lot of room blank room, the buttons are hidden.

Screencast.from.2024-10-23.08-12-22.mp4

@rissh
Copy link
Author

rissh commented Oct 23, 2024

Hi!
Thanks for your feedback. I've made the necessary adjustments to address the issue, and the buttons should now remain visible without unnecessary scrolling.

I've also added a video for reference to show how it’s working fine now.

Let me know if you have any further suggestions!
Screencast from 2024-10-23 14-56-46.webm

Copy link
Contributor

@avitova avitova left a comment

Choose a reason for hiding this comment

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

Thank you for looking into this. 🥳
If I zoom out now, the button is at the bottom. However, when zooming in, now I have to scroll. Is that the intended behaviour?

Screencast.from.2024-10-24.11-02-46.webm

@rissh
Copy link
Author

rissh commented Oct 24, 2024

Thank you for looking into this. 🥳 If I zoom out now, the button is at the bottom. However, when zooming in, now I have to scroll. Is that the intended behaviour?

Screencast.from.2024-10-24.11-02-46.webm

Hey,
I’m aware of this behavior, and I kept it that way since Ondrej suggested the buttons should always be at the bottom when zoomed out. For zooming in, the buttons stay at the bottom up to 110%, but after that, you’ll need to scroll down.

If you'd prefer the buttons to always be visible regardless of zoom, I can make that change! The only thing holding me back is that if you zoom in too much, the buttons can become annoyingly large for users. But if we want to move forward with it, I’m happy to tweak it—just say the word! 😊

@avitova
Copy link
Contributor

avitova commented Oct 24, 2024

Yes, I thought the scrolling problem was the initial reason for the issue.
My first thoughts for solving this are a bit different approach, so I thought it could be interesting to share. We could also put the contents of the custom repositories step into something like a fixed-height size "flexbox" (not sure how to describe this). That way, it would not extend and not push the button down. But honestly, that is just my two cents and different view on things. Whatever works for you!

@rissh
Copy link
Author

rissh commented Oct 25, 2024

Yes, I thought the scrolling problem was the initial reason for the issue. My first thoughts for solving this are a bit different approach, so I thought it could be interesting to share. We could also put the contents of the custom repositories step into something like a fixed-height size "flexbox" (not sure how to describe this). That way, it would not extend and not push the button down. But honestly, that is just my two cents and different view on things. Whatever works for you!

Okay, this could definitely be one approach to try. I’m happy to explore it—thanks for the suggestion! 😊

@ezr-ondrej ezr-ondrej force-pushed the fix-wizard-footer-position branch from 62538d8 to 605101d Compare November 11, 2024 09:43
Copy link
Collaborator

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

I'm still not sure how this is supposed to work, I'm lost in css positioning, could you elaborate a bit? I've left few inline comments on lines I'm mostly lost on.

@rissh
Copy link
Author

rissh commented Nov 11, 2024

I'm still not sure how this is supposed to work, I'm lost in css positioning, could you elaborate a bit? I've left few inline comments on lines I'm mostly lost on.

Sure, I'll go through each of your comments one by one and clarify the CSS positioning as best I can. I'll make any necessary adjustments along the way if needed. Thanks for the detailed feedback!

@rissh
Copy link
Author

rissh commented Nov 13, 2024

I have attached a video demonstrating the current behavior. The video showcases how the footer behaves with the latest changes and how it resolves the previous positioning issues.

Feel free to reach out if you have any questions or need further clarification! I'm still open to making any changes if needed—just let me know, and I'll be happy to update it.

Screencast.From.2024-11-13.20-00-24.mp4

@ezr-ondrej ezr-ondrej force-pushed the fix-wizard-footer-position branch from db068b9 to f0c7382 Compare November 13, 2024 14:51
Copy link
Collaborator

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

Please remove the now unecessary comments and add an empty line at EOF, apart of that it looks good to me 👍

@avitova
Copy link
Contributor

avitova commented Nov 13, 2024

Good, I am happy with this solution. Feel free to merge after Ondra approves. :)

@ezr-ondrej
Copy link
Collaborator

/ok-to-test

Copy link
Collaborator

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

Thanks 👍 Works nicely on my end.

@rissh rissh force-pushed the fix-wizard-footer-position branch from f5eb013 to 0598fb8 Compare November 19, 2024 09:58
Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 23, 2024
@ezr-ondrej ezr-ondrej force-pushed the fix-wizard-footer-position branch from 0598fb8 to 088da6a Compare December 26, 2024 19:38
Copy link
Collaborator

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

Doesn't work on Firefox

@github-actions github-actions bot removed the Stale label Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants