allo- / ffprofile

A tool to create firefox profiles with personalized defaults.
GNU Affero General Public License v3.0
769 stars 56 forks source link

Overhaul website design, drop Bootstrap & jQuery #214

Closed plungingChode closed 3 years ago

plungingChode commented 3 years ago

Use custom rendering for checkbox fields Update settings files referring to HTML tags

allo- commented 3 years ago

Thanks in advance. I need some time to review it in details before merging

For the other parts I need to test it in more detail.

plungingChode commented 3 years ago

Sure, I'll try to separate just the JSON changes. About the missing badge, sorry, but I don't really know how to add it.

allo- commented 3 years ago

The badge can be added later or somewhere as link. People should know where to find the source.

plungingChode commented 3 years ago

Oh, I see what you mean now! Maybe I accidentally removed it and forgot to put it back. I moved the links related to contribution to a new "Contribute" tab in order to keep the start tab clean.

plungingChode commented 3 years ago

Sorry about the spam, I accidentally pushed the latest commit and now I can't undo it :(

allo- commented 3 years ago

No problem.

I merged the <code> tag commit.

I tested the new design and I think the background is too bright. Experimenting in the dev-tools, I think it looks for example better to use --background-color: rgb(238, 238, 238). Then the ellipses may or may not be filled white (I am not sure, having them hollow is nice as well) and depending on how dark the new background is, neutral-main`` should probably be darker, e.g.,#484848, and I would prefer a border with more contrast for the alert, e.g.,--secondary-main: #938a00`.

Depending on if we like to treat it as information or alert, it should probably be red or the class name should be something like "info". I think I like it being yellow and there is nothing wrong with downloading a profile without answering the questions in every tab. If you like to have a look, we could also use a warn color like orange for an inline style to have a "breaks: (...)" section for settings which break website features, e.g., disabling IndexedDB.

What do you think?

plungingChode commented 3 years ago

I think you're right about the background, I toned it down to a very light gray rgb(240, 240, 240). I also took your suggestion about the contrast on --neutral-main and secondary colors (now renamed --info-* to better represent the use case), and adjusted those as well. Also adjusted are the appropriate dark theme colors.

I also added the inline warning style, but for now I used the existing --info colors, in order to keep the palette uniform. So now image

generates image and image

How about this?

allo- commented 3 years ago

Looks good. Maybe the info box is "glowing" a bit too much in the dark theme. I think it looks a bit too important there. I also didn't mean the label (but it is good to have it with some contrast), but the not yet implemented idea to render some^breaks: ["a", "b", "c"] field from the json files. (I thought we had an issue)

allo- commented 3 years ago

You didn't push your branch yet, did you?

plungingChode commented 3 years ago

No, not yet, I'm still trying to fix dark mode persistence, but can't seem to do it without using a cookie or some other server-side storage. Would that be okay/in keeping with the site's goal, or should I abandon it entirely?

Also, could you provide an example for what you have in mind for the breaks render? I just can't imagine how that would look.

allo- commented 3 years ago

It's okay. The server uses a session anyway. One could say it would be more elegant to avoid it, but when people use such a profile it will delete the cookies of this site as well ;).

See #215 for more about the idea to add "breaks" meta data.

plungingChode commented 3 years ago

I made some modifications to the options reading & rendering and this is what I came up with: image image

Of course the styling can still be modified, but just to be sure, I separated them into two different commits.

allo- commented 3 years ago

Looks good. I think details can be tuned (later) like possibly giving this addition information less contrast than the main information (especially in dark mode it seems brighter and to have more constrast) and maybe indenting it a bit. But that are again details that can still be tuned when the new theme is already online.

And the feature for this information needs to be added (and the information itself must be added to all affected settings files), so this does not need to be finished yet.

plungingChode commented 3 years ago

All right, I'll try to revert this branch to b7990a6 and open a new one for the rendering

allo- commented 3 years ago

The code tag seems to have too much padding (or the description too little). The <code> tag in the label also doesn't look good, yet.

Screenshot_20201113_232537

When testing I am also still not sure about contrast or layout. I am not sure what I am missing, but the content seems to have too little separation from the space around it. The problem may be, that the navigation now isn't separated anymore and content which is longer than the navigation is causing the effect.

Here is another issue with the description, which may be caused by the font size:

Screenshot_20201113_233000

What do you think about moving the help text into the <label> tags? This is also useful for toggling the setting by clicking the help text.

When the issues are fixed, I think we can deploy it to the site and improve on it later if there are still ideas for improving the design.

Maybe the "Contribute" Section should be "Contribute & Help" or something like this, so people know that they can report issues, e.g., to give feedback on the new site. ;)

allo- commented 3 years ago

Maybe something like this:

.navbar {
border: thin solid black;
border-radius: 1em;
display: table; /* this is just a quick hack to get a tight box, replace this with something better */
padding: 1em;
padding-right: 1em;
padding-right: 0;
background-color: #fff;
}

And "Start" shouldn't be dark blue all the time, when another page is actived. What about using bold-darkblue instead of underline for the active option?

plungingChode commented 3 years ago

I agree with you on the code tag, there was way too much vertical padding, I've now reduced it. I've also tried to separate the different parts of the option by a small margin, let me know if it seems to be enough.

About the navigation: I think it's a simple enough layout that borders shouldn't be necessary, so I increased the spacing between it and the content. Also, in keeping with the original look, I modified the hover/selection visuals (see below) image I feel like this should be enough to distinguish it from the content.

The checkbox misalignment was (I think) due to me not specifying the top offset - now added. You'd have to check this out for yourself, just to be sure (as it had looked just fine for me before as well).

However, I'm against moving the help text inside the labels - it's not exactly a part of the option itself, only a description of it. More often than not, it also contains links, which when misclicked, would cause the option to be toggled even when the user didn't mean to.

Finally, as per your request, I made the change from "Contribute" to "Contribute & help".

Let me know what you think!

allo- commented 3 years ago

About the navigation: I think it's a simple enough layout that borders shouldn't be necessary, so I increased the spacing between it and the content.

I need to see it, but probably it is better to find something without border.

I modified the hover/selection visuals

I still do not like marking start specially. And I am not sure if the highlight without background wasn't better, e.g. fat for active, maybe fat or underlines for hover and nothing special for Start.

I feel like this should be enough to distinguish it from the content.

I don't think this is the problem. The problem is, that page and navigation somehow sit inside the page without anything that marks what is padding and what is page. It looks somehow like just randomly placed on the background. I am not sure what's exactly missing, but I think there must be something what can be improved on.

The checkbox misalignment was (I think) due to me not specifying the top offset - now added. You'd have to check this out for yourself, just to be sure (as it had looked just fine for me before as well).

I thought it may need vertical-align?

However, I'm against moving the help text inside the labels - it's not exactly a part of the option itself, only a description of it. More often than not, it also contains links, which when misclicked, would cause the option to be toggled even when the user didn't mean to.

Okay. It was just an suggestion, as it seems to work better with the same margin-left (cause by the checkbox) as the title.

Let me know what you think!

You did not push it yet, did you?

plungingChode commented 3 years ago

I still do not like marking start specially.

I just thought it made sense, that the start node is always in a "valid" state, no matter where you are in the profile creation process. Take a look without that being the case:

image

To me, it looks odd, that all the other spots are filled, except for StarI still do not like marking start specially.t.

...page and navigation somehow sit inside the page without anything that marks what is padding and what is page...

Also on the image is how I propose this could be solved: push the entire page to the left, and separate the navigation and content with a line. This way I think you can clearly tell, that what's on the left is navigation and the right is content.

And I am not sure if the highlight without background wasn't better, e.g. fat for active, maybe fat or underlines for hover and nothing special for Start.

I was just thinking that with the darker color of the inactive options it's kind of hard to tell, which is active, if I'm marking the valid ones like so:

image

If anything, it's easier to tell without the valid ones having bold font, but a full circle next to them:

image

(This way, the Start node also doesn't look so out of place being empty)

thought it may need vertical-align?

I've added it now, seems like it's working. But check it out when you get the chance.

You did not push it yet, did you?

I'm only going to push the changes, when you agree to the design :)

allo- commented 3 years ago

I just thought it made sense, that the start node is always in a "valid" state, no matter where you are in the profile creation process. Take a look without that being the case:

Now I see your point. It should probably be bold like the other bullet points: Active when the page is finished, i.e., the user chose a profile.

Also on the image is how I propose this could be solved: push the entire page to the left, and separate the navigation and content with a line.

I thought about it, this may be a good option.

I was just thinking that with the darker color of the inactive options it's kind of hard to tell, which is active, if I'm marking the valid ones like so

Bold for finished is fine. Underlines for active maybe. Maybe just marking it by color? On the other hand it is always good not to use only color. The circle is a good indicator in any case.

Contribute should probably not have a circle, but on the other hand it would not line up properly. Maybe the circle in another color, shape, ...? I don't know without testing an design. Maybe it is just fine.

I'm only going to push the changes, when you agree to the design :)

It's good to be able to test it myself. Especially testing it also with my browser profile that has settings like a minimum font size, which my break some designs, which assume pixel perfect rendering.

plungingChode commented 3 years ago

Ok, I tried to include all the requested changes. Please tell me if anything's missing.

allo- commented 3 years ago

This looks much better now!

The Github Button is very nice. The checkboxes are still not correctly aligned here. The .chk label has top/right margin and the text right of it does not have any margin. And I am not sure if the helptext should not be indented when using checkboxes to make it easier to see that it really belongs to the option.

image

I think I know what's wrong with the Start button: It doesn't change its state when you start. It should be like the non-saved pages until you press start. you can probably just check if {{ active_profile }} is set.

plungingChode commented 3 years ago

you can probably just check if {{ active_profile }} is set.

You're right, making it depend on that value did the trick. However, now it remains empty after a reset if the start button isn't explicitly pressed, even when a section has been saved. Is there any way to reapply the current profile and therefore the active_profile?

The checkboxes are still not correctly aligned

This time I tried to use the same method as in the current Bootstrap version. Please tell me how it looks.

allo- commented 3 years ago

Screenshot_20201114_171348

allo- commented 3 years ago

The problem is that the default profile is used for active_profile="" or none.

Try to change the main method like this:

    if not "profile" in request.session:
        request.session["profile"] = sorted(PROFILES)[0]
    form_classes = PROFILES.get(request.session.get("profile"), ["empty", []])[1]

Another option would be not to load a default profile but require the user to press start. There is quite a bit of work I would like to do for the profile infrastructure. For example, selecting multiple profiles, e.g., one for privacy and one for UI/UX settings, which are then merged. I appreciate feedback when you like to change how the web interface handles profiles.

plungingChode commented 3 years ago

I changed the "next" post handling somewhat instead, now it properly updates when clicking "Save & next", but not on "Save". I'll look into it some more.

Meanwhile, can you check the checkbox alignment? I tried to remove the top margin, but I'm running out of ideas :/

plungingChode commented 3 years ago

OK, I think with 80b70d9 I fixed the start node not updating, and with 8e826e3 the checkbox alignment issues.

allo- commented 3 years ago

I am not sure what's still missing or if the PR can be merged.

@plungingChode Can you have a look at the bootstrap messages template-tag and then rebase and squash the commits in your branch as you like them to look like, so I can merge it?

qx-775 commented 3 years ago

Any status on this PR? Design looks really good!. It has been a few months since the last question. Don't want to force anyone. Just curious, maybe giving a little kickstart to get it merged.

plungingChode commented 3 years ago

Oh, I might have forgotten about this one completely. Sorry about that! I'll have a look at it this week.

plungingChode commented 3 years ago

I checked the bootstrap-messages thing: since the project doesn't use flash messages, it's not necessary. I also squashed the commits as requested, with one for the actual feature and one to include the changes since the last commit. As far as I can see, it's ready to merge.

qx-775 commented 3 years ago

Nice dude!

allo- commented 3 years ago

Merged and installed on the production site. Opened #225 for displaying a fallback message when the main script is blocked.

allo- commented 1 year ago

@plungingChode Would you like to have a look at #261?

I think it may be possible to reduce the script and maybe even get completely rid of it. I wonder what are your thoughts on it. I guess the dark mode would require a server-side switch (storing it in the session and adding a line to a <style> block, but changing tabs works out of the box and I guess the transitions can be done in pure CSS as well.