martignoni / hugo-notice

A Hugo theme component to display nice notices
GNU General Public License v3.0
236 stars 48 forks source link

Simplify color settings #36

Closed rea1shane closed 3 months ago

rea1shane commented 6 months ago

Seems no bug after update.

Light theme

image

Dark theme

image

Source code

## Test

{{< notice note >}}
This is a very good note.
{{< /notice >}}

{{< notice info >}}
This is a very good info.
{{< /notice >}}

{{< notice tip >}}
This is a very good tip.
{{< /notice >}}

{{< notice warning >}}
This is a warning notice. Be warned!
{{< /notice >}}

{{< notice Others >}}
Non-default type.
{{< /notice >}}
martignoni commented 6 months ago

Can you please split your PR to solve only the minifying?

I've the policy to have each issue managed with a unique PR. Do not hesitate to create an issue and other PRs for your color simplification proposition and other issues.

martignoni commented 6 months ago

Also: please do not change the CSS or other things, just unminify it (for this PR).

rea1shane commented 6 months ago

Also: please do not change the CSS or other things, just unminify it (for this PR).

All right, see https://github.com/martignoni/hugo-notice/pull/37

rea1shane commented 6 months ago

Conflict resolved, please review and check it. @martignoni

martignoni commented 6 months ago

Could you please explain why these simplifications?

rea1shane commented 6 months ago

Could you please explain why these simplifications?

This PR has made six modifications:

  1. Since notice defaults to note and is too similar to note when the user sets noticeType to something other than the preset (which, by the way, doesn't look right on the current main branch in dark mode), note and the non-preset values are set to the same color and are used as the base color for notice.

  2. Previously, adding a new preset required modifying five places:

    .notice {
       --new-title: #5a5;
       --new-content: #efe;
    }
    
    @media (prefers-color-scheme:dark) {
       .notice {
           --new-title: #363;
           --new-content: #121;
       }
    }
    
    body.dark .notice {
       --new-title: #363;
       --new-content: #121;
    }
    
    .notice.new .notice-title {
       background: var(--new-title)
    }
    
    .notice.new {
       background: var(--new-content)
    }

    now it only requires three:

    .notice.new {
       --title-background-color: #fb7;
       --content-background-color: #fec;
    }
    
    @media (prefers-color-scheme:dark) {
       .notice.new {
           --title-background-color: #a50;
           --content-background-color: #420;
       }
    }
    
    body.dark .notice.new {
       --title-background-color: #363;
       --content-background-color: #121;
    }

    And it reads more clearly.

  3. Change $.Page.Scratch.Add to $.Page.Scratch.Set. Because it's simpler and we just need a flag. This is Hugo's source code:

    // Add will, for single values, add (using the + operator) the addend to the existing addend (if found).
    // Supports numeric values and strings.
    //
    // If the first add for a key is an array or slice, then the next value(s) will be appended.
    func (c *Scratch) Add(key string, newAddend any) (string, error) {
        var newVal any
        c.mu.RLock()
        existingAddend, found := c.values[key]
        c.mu.RUnlock()
        if found {
            var err error
    
            addendV := reflect.TypeOf(existingAddend)
    
            if addendV.Kind() == reflect.Slice || addendV.Kind() == reflect.Array {
                newVal, err = collections.Append(existingAddend, newAddend)
                if err != nil {
                    return "", err
                }
            } else {
                newVal, err = math.DoArithmetic(existingAddend, newAddend, '+')
                if err != nil {
                    return "", err
                }
            }
        } else {
            newVal = newAddend
        }
        c.mu.Lock()
        c.values[key] = newVal
        c.mu.Unlock()
        return "", nil // have to return something to make it work with the Go templates
    }
    
    // Set stores a value with the given key in the Node context.
    // This value can later be retrieved with Get.
    func (c *Scratch) Set(key string, value any) string {
        c.mu.Lock()
        c.values[key] = value
        c.mu.Unlock()
        return ""
    }
  4. There is no need to operate style loaded flag every time when shortcode is called, just first time.

  5. Remove first class before notice-title. It seems useless.

  6. Added some missing ;.

martignoni commented 6 months ago

3. Change $.Page.Scratch.Add to $.Page.Scratch.Set

This needs its own PR (not related to color simplification).

5. Remove first class before notice-title. It seems useless.

This too.

rea1shane commented 6 months ago
  1. Change $.Page.Scratch.Add to $.Page.Scratch.Set

This needs its own PR (not related to color simplification).

  1. Remove first class before notice-title. It seems useless.

This too.

I will do this tonight.

martignoni commented 6 months ago

(…) which, by the way, doesn't look right on the current main branch in dark mode

Could you open an issue for that, since it looks like a bug?

rea1shane commented 6 months ago

(…) which, by the way, doesn't look right on the current main branch in dark mode

Could you open an issue for that, since it looks like a bug?

Sure.

rea1shane commented 6 months ago

Fix #39