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

Color updates #19

Merged
merged 6 commits into from
Dec 29, 2020
Merged

Color updates #19

merged 6 commits into from
Dec 29, 2020

Conversation

drbyte
Copy link

@drbyte drbyte commented Dec 25, 2020

No description provided.

@drbyte
Copy link
Author

drbyte commented Dec 25, 2020

There may be more areas to refine in regard to colors.
I'm okay with this remaining unmerged until we can review more.

@lat9
Copy link
Owner

lat9 commented Dec 26, 2020

Issue #21 opened to track these updates.

@dennisns7d
Copy link

While you're in this file, please review the header category tabs section. It appears to me that ZCA_HEADER_TABS_COLOR and ZCA_HEADER_TABS_COLOR_HOVER appear to have been swapped (perhaps since day one). Shouldn't ZCA_HEADER_TABS_COLOR apply to #navCatTabs a and ZCA_HEADER_TABS_COLOR_HOVER apply to #navCatTabs a:hover?

@drbyte
Copy link
Author

drbyte commented Dec 28, 2020

While you're in this file, please review the header category tabs section. It appears to me that ZCA_HEADER_TABS_COLOR and ZCA_HEADER_TABS_COLOR_HOVER appear to have been swapped (perhaps since day one). Shouldn't ZCA_HEADER_TABS_COLOR apply to #navCatTabs a and ZCA_HEADER_TABS_COLOR_HOVER apply to #navCatTabs a:hover?

Are you saying this because visually it looks wrong? Or because text-descriptions mentally seem misaligned?

I'm having trouble seeing what you think it should look like visually.

@dennisns7d
Copy link

The text-descriptions seem misaligned.

I just looked at the admin side and found the same misalignment, so the two misalignments cancel out. Visually they are OK.

@dennisns7d
Copy link

The text-descriptions seem misaligned.

I just looked at the admin side and found the same misalignment, so the two misalignments cancel out. Visually they are OK.

My old eyes don't always work. Visually they're swapped between admin and store sides.

@dennisns7d
Copy link

admin/includes/init_includes/init_bc_config.php sets the following titles for the admin side:
ZCA_HEADER_TABS_COLOR: Header Category Tabs Color
ZCA_HEADER_TABS_COLOR_HOVER: Header Category Tabs Color on Hover
So the admin side is aligned.

The store side is reversed. Looking at the color values on the admin side, the Color on Hover is a lower value than Color. The Color on Hover is darker than Color. Looking at the store side, the color on hover is lighter than non-hover.

@drbyte
Copy link
Author

drbyte commented Dec 28, 2020

You're right. It's reversed, and the default colors are therefore reversed too.

drbyte added a commit to drbyte/ZCA-Bootstrap-Template that referenced this pull request Dec 28, 2020
@drbyte
Copy link
Author

drbyte commented Dec 28, 2020

I've added the following to this PR:
Screen Shot 2020-12-28 at 3 49 42 PM

and PR'd an update to swap the default colors and improve the admin descriptions of those values: https://github.com/lat9/ZCA-Bootstrap-Template/pull/25/files

drbyte added a commit to drbyte/ZCA-Bootstrap-Template that referenced this pull request Dec 28, 2020
@drbyte
Copy link
Author

drbyte commented Dec 28, 2020

If I've done it right, we might want to add something in the Release Notes that tells people who are upgrading to double-check (and possibly swap) the colors they've assigned for Header Category Tabs Color and Header Category Tabs Color on Hover, if they even have Category Tabs enabled.

drbyte added a commit to drbyte/ZCA-Bootstrap-Template that referenced this pull request Dec 28, 2020
Copy link
Owner

@lat9 lat9 left a comment

Choose a reason for hiding this comment

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

Should we now include a stylesheet_configs.php (I'm sure there's a better name) to include these non-color-specific styling changes?

@drbyte
Copy link
Author

drbyte commented Dec 29, 2020

Should we now include a stylesheet_configs.php (I'm sure there's a better name) to include these non-color-specific styling changes?

IMO no.

In theory end-users won't "need" to touch/edit this file, so if it contains one entry that's not color-related but pulls dynamic values from store configuration, it won't cause any confusion.
Plus, less-is-more performance-wise when it comes to loading assets such as stylesheets. The fewer the better.

If the current naming of stylesheet_colors.php is confusing in terms of semantics, I have no objection to it being renamed.

@lat9 lat9 merged commit 372b1ee into lat9:v300 Dec 29, 2020
@drbyte drbyte deleted the colors branch December 29, 2020 16:28
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

3 participants