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

[Improvement - LOW] Some minor corrections to the new Snow theme #105

Closed
wants to merge 11 commits into from

Conversation

pupi1985
Copy link
Contributor

@pupi1985 pupi1985 commented Oct 6, 2014

If they are not clear then let me know and I can provide a before and after screenshot.

By the way, I'm not so sure it is a good idea to name the new Snow theme the same way as the old one. I'm thinking about someone making a plugin that checks for the Snow theme and then matching a different one.

I also think applying borders to every td is too much:

table tbody tr td {
    padding: 10px 0;
    border-top: 1px solid #ecf0f1;
    border-bottom: 1px solid #ecf0f1;
    display: table-cell;
}

Particularly after seeing how wide forms with labels are being displayed (horizontal lines both up and down on labels). Looks like it is too dense in horizontal lines. This includes plugins, profile section, admin permissions, etc. Not to mention the label for a tall checkbox does not have that because it is applies border: none in td.qa-form-tall-label so it is not always with the lines either so checkboxes for tall or wide make the impression that they are separators.

I'm attaching some screenshots of these 2 commits:

  • Allowed new sections to be added to the right side of the profile section
  • Removed the unnecessary white space in the profile section

Current state for a user profile that results in a lot of white space after the type field:
1

After removing the minimal height we can see the profile section is holding the components on the right, which will be annoying when trying to add new sections to the theme:
2

After the fix:
3

Note the minimal height can be kept and the new sections will still be aligned because of the clear. However, I think the reason why the minimal height was there was to hold the components on the right.

@svivian
Copy link
Collaborator

svivian commented Oct 15, 2014

Thanks for this. Unfortunately the CSS is currently being generated via SASS (which Jatin did not wish to publish, and I didn't really want in the repo anyway). But not to worry, I'm currently applying some of these manually in the SASS code.

I have changed the theme name to SnowFlat for now since there is still some work to do and I want to get the beta out the door.

I agree about the table borders, I've removed the generic selector and put them on the wide table only.

There are some other problems with the SASS code at the moment (such as too much unnecessary nesting, causing bloated selectors) so I will try and clean it up asap to the point where we can directly edit the CSS going forward.

@svivian
Copy link
Collaborator

svivian commented Oct 15, 2014

OK now fixed all this in new commits. Thanks again.

@svivian svivian closed this Oct 15, 2014
@pupi1985
Copy link
Contributor Author

Great. I'll update my repo, take a look at this and report back if I notice any issue. Thanks.

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.

None yet

2 participants