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

Vue fields: add default CSS class for field type #5009

Closed
Daandelange opened this issue Jan 17, 2023 · 3 comments · Fixed by #6604
Closed

Vue fields: add default CSS class for field type #5009

Daandelange opened this issue Jan 17, 2023 · 3 comments · Fixed by #6604
Assignees
Labels
needs: discussion 🗣 Requires further discussion to proceed
Milestone

Comments

@Daandelange
Copy link

Hello, it's me extending fields again. :p

Description

This time I tried to extend a field without rewriting its template (I have no need for it).
So the template of my field is the same as the base field.
Then I tried to inject some custom panel css for my field, but it's got the classname of the base field : k-text-field.
I don't want to change the base field too.

Expected behaviour
I expect the fields' class naming to be consistent, thus enforcing the class of the field type.
Maybe even "accumulated inherited classes", to preserve the css of the initial field ?
In this case I'd expect either: Sorry if the is wishful thinking (and thus rather a feature request).

  1. class="k-field k-customcss-field k-field-name-myfield"
  2. class="k-field k-customcss-field k-text-field k-field-name-myfield"

I have looked in Field.vue where it doesn't seem to enforce the type class.
The different FieldType.Vue that I checked then have their own hardcoded classes : k-text-field for example.

To reproduce

<?php
// File: site/plugins/customcss/index.php
Kirby::plugin('kirbybugs/customcss', [
    'customcss' => [
        'extends' => 'text',
    ],
]);
// File : site/plugins/customcss/index.js
panel.plugin('kirbybugs/customcss', {
    fields: {
        customcss: {
            extends: "k-text-field",
        },
    },
});
# In a blueprint
fields:
  myfield:
    type: customcss

Go to the panel and observe the HTML class of the field not containing k-customcss-field. (use your dev tools)
Inspecting the Vue component : My custom component is loaded and its type prop is correct : customcss.

Your setup

Kirby Version 3.8.4.

@Daandelange Daandelange changed the title Fiber field-class incorrect for extended fields Fiber field-css-class incorrect in extended fields with inherited template Feb 7, 2023
@distantnative
Copy link
Member

@Daandelange I don't think this is a bug. If you are completely inheriting the Vue template of the text field, of course all the HTML output will be that of a text field. How would Vue know that you would want to alter the inherited HTML output if you aren't altering it at all?

The only thing I could imagine here is that Field.vue not only adds 'k-field k-field-name-' + name by default but also 'k-field-type-' + type. Then you would end up with something like

class="k-field k-field-type-customcss k-text-field k-field-name-myfield"

@distantnative distantnative added the needs: discussion 🗣 Requires further discussion to proceed label Feb 25, 2023
@distantnative distantnative changed the title Fiber field-css-class incorrect in extended fields with inherited template Vue fields: add default CSS class for field type Feb 25, 2023
@distantnative distantnative self-assigned this Feb 25, 2023
@Daandelange
Copy link
Author

To me, this issue hits right into the one major drawback of the Vue components system : you cannot call inherited parent methods if you specialise them. Specialising = Overwriting. For JS functions I found a hacky workaround, but for templates it doesn't seem to be possible.

I don't think this is a bug. If you are completely inheriting the Vue template of the text field, of course all the HTML output will be that of a text field. How would Vue know that you would want to alter the inherited HTML output if you aren't altering it at all?

From a Vue perspective, yes, but I think it's more a kirby-base-field-component related logic that should overcome the Vue limitations. When I extend a panel field, I expect it to preserve it's original look and functionality (unless explicitly modified). As Kirby uses some kind of "automatic css class name generator from code objects", I think it'd feel natural to have that extra child name included along with the parent names.

Yes, adding the field's type prop as dynamic CSS property, is my suggestion too.
Note that the hardcoded/static class name must remain present in all fields to ensure inheriting that too when extending them (and thus replacing type). So in most cases the static and dynamic ones will be identical, Vue outputting only one of them. The dynamic one is there for custom fields with inherited templates to ensure css-class-name-consistency.


Here's a sum-up overview :

<!-- TextField.Vue (or any other native field impl) -->
<template>
    <!-- Inject "static" class name, k-field will inject the "dynamic" one -->
    <k-field class="k-text-field">
        <!-- Field code... -->
    </k-field>
<template>
<!-- Field.Vue -->
<template>
    <!-- CSS consistency : inject dynamic field-type and field-name  -->
    <div :class="'k-field k-field-name-' + name + ' k-field-type-' + type">
        <!-- Field code... -->
    </div>
<template>

Now, inheriting k-text-field, my customcss field will have both my custom field type and the inherited class type. I am now inheriting the CSS of the original text field and I can also provide extra custom CSS for my custom field.

<div class="k-field k-field-type-customcss k-text-field k-field-name-myfield"></div>

@Daandelange
Copy link
Author

Awesome, thanks ! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: discussion 🗣 Requires further discussion to proceed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants