Skip to content

New Home Page #842

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

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

New Home Page #842

wants to merge 29 commits into from

Conversation

blazgocompany
Copy link

I will make more styles soon but for now its just a nicer home page.

@cally-jt
Copy link
Member

Could you provide a picture...?

@blazgocompany
Copy link
Author

blazgocompany commented Jun 21, 2024

I cannot test what it looks like but maybe you can. I know it looks good...

@MaterArc
Copy link
Member

MaterArc commented Jun 21, 2024

Screenshot 2024-06-21 at 5 15 42 PM

Could you provide a picture...?

The hover effect is nice, but the colors could be improved...

@cally-jt
Copy link
Member

How did you make it if you didnt test it? And no i cannot provide one since i dont have access to a pc rn

@cally-jt
Copy link
Member

Screenshot 2024-06-21 at 5 15 42 PM

Could you provide a picture...?

The hover effect is nice, but the colors could be improved...

Ok yea this should be improved before possible merge

@blazgocompany
Copy link
Author

Wait somethings wrong..

@MaterArc
Copy link
Member

MaterArc commented Jun 21, 2024

Screenshot 2024-06-21 at 5 15 42 PM > Could you provide a picture...?

The hover effect is nice, but the colors could be improved...

Ok yea this should be improved before possible merge

For sure, yeah. I assume this isn't the finished result. I also added the white text color to the css before injecting it because before the contrast wasn't the best.

@cally-jt
Copy link
Member

cally-jt commented Jun 21, 2024

Possible ideas

  • make each section a different colour instead of 1 bland orange
  • add a bigger contrast between the "view all" button and background

@blazgocompany
Copy link
Author

blacks more readable on orange

@blazgocompany
Copy link
Author

add a bigger contrast between the "view all" button and background
add "!important" on line 19. The View all should be black

@blazgocompany
Copy link
Author

blazgocompany commented Jun 21, 2024

View Image ![image](https://github.com/STForScratch/ScratchTools/assets/96586789/d82be5a0-0451-4697-aa4f-70efee547fc0) Black on orange ![image](https://github.com/STForScratch/ScratchTools/assets/96586789/5b18a66f-290b-42a4-a3a3-d1f6f6323220) White on orange

@cally-jt
Copy link
Member

image

Black on orange

image

White on orange

It isnt white tho, its a dark blue which id even more unreadable

@blazgocompany
Copy link
Author

The view all button and the box header should both be black. My original code was contrast tested

@MaterArc
Copy link
Member

MaterArc commented Jun 21, 2024

The view all button and the box header should both be black. My original code was contrast tested

Fair enough but the only reason I changed it then and there was because when I saw it it was:

Screenshot 2024-06-21 at 5 25 59 PM

@blazgocompany
Copy link
Author

Thats weird

@MaterArc
Copy link
Member

MaterArc commented Jun 21, 2024

Thats weird

No, you're right. I was just showcasing that the color it currently is should be darker (black) which you already noted as it's currently a shade of gray which looks off. So, just change the color to black and from a color standpoint it should be good 👍

@blazgocompany
Copy link
Author

ok I see its gray.

@blazgocompany
Copy link
Author

I had put color:black i dont see it anymore..

@blazgocompany
Copy link
Author

Done! See line 19 & 16

@MaterArc
Copy link
Member

Screenshot 2024-06-21 at 5 32 27 PM

Yep, it's better now

@blazgocompany
Copy link
Author

Is that done?

@cally-jt
Copy link
Member

I think this could work as an experimentational feature for now.

Would need others opinions

@MaterArc
Copy link
Member

Like I said, it needs work! But it's definitely a start...

@rgantzos rgantzos added the new feature Adds a new feature to the settings page label Jun 21, 2024
@blazgocompany
Copy link
Author

@MaterArc Hmm.. Ok thanks.

@blazgocompany
Copy link
Author

@MaterArc @rgantzos Can you test the profile page and tell me what you think. I don't know how to use JS in extensions. Hope I did it correctly.

@blazgocompany
Copy link
Author

Can we please approve this PR quickly. I would like to role out these features now while I work to add more.

@cally-jt
Copy link
Member

Can we please approve this PR quickly. I would like to role out these features now while I work to add more.

Would need rg approval

If we were to merge this rn it should be a beta feature

@blazgocompany
Copy link
Author

Thanks for responding. Yes, BETA feature is fine with me!

@blazgocompany
Copy link
Author

Did you check out the profile page styles.

@cally-jt
Copy link
Member

Did you check out the profile page styles.

I cannot rn, in the future please provide pictures when possible

@blazgocompany
Copy link
Author

Hello????? I am wondering if we can merge this, and if not why? @rgantzos @MaterArc @callumjt

@rgantzos
Copy link
Collaborator

rgantzos commented Jul 3, 2024

Hello????? I am wondering if we can merge this, and if not why? @rgantzos @MaterArc @callumjt

It doesn't seem to be customizable... that's definitely something that could expand use cases. At the moment I don't see many.

@blazgocompany
Copy link
Author

Hmm.. Ok.

At the moment I don't see many

Don't see many what?
And are the customizability features blocking this PR from being merged or is that just a "good-to-have" feature?

@rgantzos
Copy link
Collaborator

rgantzos commented Jul 3, 2024

Hmm.. Ok.

At the moment I don't see many

Don't see many what? And are the customizability features blocking this PR from being merged or is that just a "good-to-have" feature?

At the moment, this userstyle is rather niche and would only benefit a small margin of users. However, if it was customizable, it would be much more useful to more users.

@blazgocompany
Copy link
Author

blazgocompany commented Jul 3, 2024

@rgantzos Got it. What would you like to add as customizable features. I would expect the dark/light themes. But what else? Also I didn't find @STForScratch/website-developers but I did find website, website2, etc..

@blazgocompany
Copy link
Author

@rgantzos Maybe you or @MaterArc could help me with the customizing. I haven't done something like this before, but once I know how to, I'll be fine.

@MaterArc
Copy link
Member

MaterArc commented Jul 4, 2024

@rgantzos Maybe you or @MaterArc could help me with the customizing. I haven't done something like this before, but once I know how to, I'll be fine.

Hey! I might be able to help you once I'm back to my computer, I'm leaving the country soon and won't be back until Saturday morning

@blazgocompany
Copy link
Author

Ok! Thanks!

@blazgocompany
Copy link
Author

@MaterArc I need to add a toggle switch to the feature JSON, which I think I have done correctly. The only problem is how I am supposed to get that as a CSS variable that allows me to change the Dark/Light mode of the background

@MaterArc
Copy link
Member

MaterArc commented Jul 8, 2024

@MaterArc I need to add a toggle switch to the feature JSON, which I think I have done correctly. The only problem is how I am supposed to get that as a CSS variable that allows me to change the Dark/Light mode of the background

You're going to have to use some JavaScript

@blazgocompany
Copy link
Author

@MaterArc do you know exactly what I need to do?

@blazgocompany
Copy link
Author

Hello???

*echoes*

Anyone Here???

@MaterArc @rgantzos

I think this is quite a long time to have an open PR. Can we merge this soon?

@MaterArc
Copy link
Member

Hello???

echoes

Anyone Here???

@MaterArc @rgantzos

I think this is quite a long time to have an open PR. Can we merge this soon?

Doesn't seem like this is complete enough for a merge. I can test it out and drop all of my feedback here.

@rgantzos rgantzos self-requested a review August 18, 2024 01:09
@blazgocompany
Copy link
Author

@MaterArc @callumjt @rgantzos, I really hate to spam, but even if this is not complete enough for a merge, is there something I need to do to make it complete? Or is this just not planned?

@blazgocompany
Copy link
Author

Also I can see that there are merge conflicts and I don't quite know how to fix those. Maybe one of you can help me with that.

"scripts": [{"file": "scratch-www.js", "runOn": "/"}, { "file": "scratch-profile.js", "runOn": "/users/*" }],
"tags": ["Theme", "New"],
"type": ["Website", "Editor"],
"options": [{ "id": "theme", "name": "Mode", "type": 0 }],
Copy link
Collaborator

Choose a reason for hiding this comment

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

You have an option yet it is never referenced anywhere.

Copy link
Author

@blazgocompany blazgocompany Aug 23, 2024

Choose a reason for hiding this comment

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

See scratch-www.js line 4:
ScratchTools.Storage.theme
Is that how you reference an option, or do I do something different?

Copy link
Author

@blazgocompany blazgocompany Aug 23, 2024

Choose a reason for hiding this comment

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

@rgantzos Also, instead of integrating dark mode into my "tool" why can't I (or someone else) just make another dark mode tool that is compatible with this?

@blazgocompany
Copy link
Author

Hi, @rgantzos @cally-jt @MaterArc, This PR is left open and unmerged for more than a month. If this PR is not required, not well-designed, not "welcome" to this extension, or just simple not detailed enough, I can close this. Let me know if you still believe that this PR can be merged to the extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature Adds a new feature to the settings page scope: feature status: needs review Waiting for further review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants