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

Fixes Sidebar editor repetition count for "days" #6551

Merged
merged 4 commits into from
Dec 8, 2024
Merged

Conversation

keremsemiz
Copy link
Contributor

@keremsemiz keremsemiz commented Dec 5, 2024

Fixes #6429

Adjusted CSS for proper centering of the Repeat section in the sidebar at low px counts.
image

@keremsemiz keremsemiz added the 3. to review Waiting for reviews label Dec 5, 2024
@keremsemiz keremsemiz requested a review from GretaD December 5, 2024 12:02
@keremsemiz keremsemiz self-assigned this Dec 5, 2024
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 23.44%. Comparing base (7f70db9) to head (dc74e7c).
Report is 21 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #6551   +/-   ##
=========================================
  Coverage     23.44%   23.44%           
  Complexity      472      472           
=========================================
  Files           249      249           
  Lines         11888    11888           
  Branches       2282     2271   -11     
=========================================
  Hits           2787     2787           
  Misses         8774     8774           
  Partials        327      327           
Flag Coverage Δ
javascript 14.87% <ø> (ø)
php 59.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@keremsemiz keremsemiz changed the title Fixes Sidebar editor repetition count for "days" is not put on separa… Fixes Sidebar editor repetition count for "days" #6429 Dec 5, 2024
@SebastianKrupinski
Copy link
Contributor

Tested. Works.

But it does not seem to inline when the screen is large enough.

image

@GretaD
Copy link
Contributor

GretaD commented Dec 5, 2024

yes, @keremsemiz the "repeat every" field should be smaller, so both input field can fit in one row, because the content they hold is very small. So remove the flex-direction: column for that.

The second part: end repeat.
The second field(the date one) should not always be positioned below the first field, should only slide below if theres no space. As you can see on the Sebastian's screen, the fields are still above each other even though theres space. It should be next to each other.

What you can do is, have a set width for the fields, and wrap them in a flex box to move below if theres no space.

@keremsemiz
Copy link
Contributor Author

The second part: end repeat. The second field(the date one) should not always be positioned below the first field, should only slide below if theres no space. As you can see on the Sebastian's screen, the fields are still above each other even though theres space. It should be next to each other.

What you can do is, have a set width for the fields, and wrap them in a flex box to move below if theres no space.

Yes, I understand what you mean. Originally I did it that way but I thought that the issue always needed them to be on seperate lines 👍

@GretaD
Copy link
Contributor

GretaD commented Dec 5, 2024

The second part: end repeat. The second field(the date one) should not always be positioned below the first field, should only slide below if theres no space. As you can see on the Sebastian's screen, the fields are still above each other even though theres space. It should be next to each other.
What you can do is, have a set width for the fields, and wrap them in a flex box to move below if theres no space.

Yes, I understand what you mean. Originally I did it that way but I thought that the issue always needed them to be on seperate lines 👍

no problem, try this new approach when you have the time

@keremsemiz
Copy link
Contributor Author

I've went over the issues again, here is the revised version:
image

@SebastianKrupinski
Copy link
Contributor

SebastianKrupinski commented Dec 5, 2024

Hey, tested again the counts is now properly in line in all screen sizes.

But the On Date still shows on two lines no matter the screen size.

image

@keremsemiz
Copy link
Contributor Author

keremsemiz commented Dec 5, 2024

Thank you for pointing that out, I'll make sure to fix it tomorrow.

@ChristophWurst ChristophWurst changed the title Fixes Sidebar editor repetition count for "days" #6429 Fixes Sidebar editor repetition count for "days" Dec 6, 2024
@GretaD
Copy link
Contributor

GretaD commented Dec 6, 2024

i checked it a bit myself, and i cannot find a reliable way to make it work that always fits the space and when no space to slide down. So i suggested Kerem to make it like this:

Screenshot from 2024-12-06 11-47-43

@SebastianKrupinski
Copy link
Contributor

Tested. Again. Looks good to me, probably the best we going to get.

On Date
image

On Count
image

@SebastianKrupinski
Copy link
Contributor

SebastianKrupinski commented Dec 6, 2024

@keremsemiz can you squash all the commits in to one and rebase please

@keremsemiz keremsemiz merged commit 5fa4abb into main Dec 8, 2024
38 checks passed
@keremsemiz keremsemiz deleted the keremsemiz branch December 8, 2024 13:04
@keremsemiz
Copy link
Contributor Author

yep, did it.

Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

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.

Sidebar editor repetition count for "days" is not put on separate line
3 participants