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

added a notice about saving changes first before testing #1257

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

Conversation

ydkmlt84
Copy link
Collaborator

@ydkmlt84 ydkmlt84 commented Sep 7, 2024

I personally thought the words Test SAVED were clarification enough, but there are a lot of people having this issue. Figured I could add a little helper notice. Also, my linter settings (or something) must be different than yours because it is always automatically adding those indentation changes to my commits.

I am going to attempt to put it back to the way you had it.

@ydkmlt84
Copy link
Collaborator Author

ydkmlt84 commented Sep 7, 2024

#1221

@jorenn92
Copy link
Owner

jorenn92 commented Sep 9, 2024

Hey, thanks for the PR! I'm first going to release a smaller 2.1.1 release that addresses #1250 & #1242, since this broke after 2.1.0. (Probably later today). After that i'll merge this one.

@jorenn92
Copy link
Owner

I'm not completely satisfied with how the design looks on mobile. In the past, I kept the button text short to prevent the buttons from appearing too large on smaller screens, but now they look oversized:

image

Would renaming "Add Rule to Section" to "Add to Section" and "Add a New Section" to "Add New Section" be a good compromise? I also feel that the "Community Rules" button is too large. While just using "Community," as I did before, feels too vague, perhaps there's a better alternative, though I'm not sure what that would be.

As for the "Test Saved" notification, it’s not working well on mobile either:

Before:
image

Now:
image

A potential solution could be placing the notification between the "Docs" and "Test" buttons, utilizing the full width, and restoring the "Test" buttons to their original size.

Also, if I may nitpick, I'd prefer the notification's text to be centered to the buttons in the desktop view as well. It's a tiny bit too high now:
image

@ydkmlt84
Copy link
Collaborator Author

I agree with all of those things. As I have done before when making the Maintainerr.info website, I am not good with mobile. My main goal was clarification in everything, coming from common issues I've seen in discord.

I will take a look at this today and see if I can come up with some better ideas/options. I will fix the alignment of the test saved notification on desktop view, and see if putting it above the buttons fixes it for mobile view.

@jorenn92
Copy link
Owner

I agree with all of those things. As I have done before when making the Maintainerr.info website, I am not good with mobile. My main goal was clarification in everything, coming from common issues I've seen in discord.

I will take a look at this today and see if I can come up with some better ideas/options. I will fix the alignment of the test saved notification on desktop view, and see if putting it above the buttons fixes it for mobile view.

Thanks! If you run into any issues, just hit me up on discord. I'd be happy to help.

@ydkmlt84
Copy link
Collaborator Author

ydkmlt84 commented Sep 14, 2024

How do you feel about these changes? I swapped the test and save buttons so that left-to-right is Save first and then Test. I completely removed the text about saving first. I could get it to look fine in mobile and then it would be messed up in desktop. I think this should help clarify saving first since it's the first button when reading left to right.

Screenshot 2024-09-13 233203
Screenshot 2024-09-13 233134

For the rules buttons I made some changes too, and the Community button now says Shared. Two words of anything would make the button too big in mobile. Maybe Shared is a little less vague than Community?

Screenshot 2024-09-13 233100
Screenshot 2024-09-13 233850
Screenshot 2024-09-13 233043

@jorenn92
Copy link
Owner

How do you feel about these changes? I swapped the test and save buttons so that left-to-right is Save first and then Test. I completely removed the text about saving first. I could get it to look fine in mobile and then it would be messed up in desktop. I think this should help clarify saving first since it's the first button when reading left to right.

Screenshot 2024-09-13 233203 Screenshot 2024-09-13 233134

For the rules buttons I made some changes too, and the Community button now says Shared. Two words of anything would make the button too big in mobile. Maybe Shared is a little less vague than Community?

Screenshot 2024-09-13 233100 Screenshot 2024-09-13 233850 Screenshot 2024-09-13 233043

This looks a lot better! I'm not really sold on the 'shared' button name though. Perhaps something like 'Hub'? Although that might be too vague as well.

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.

2 participants