feat: create BaseButton component #1123
No reviewers
Labels
No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
No Milestone
No project
3 Participants
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vikunja/frontend#1123
Loading…
Reference in New Issue
No description provided.
Delete Branch "dpschen/frontend:feature/feat-BaseButton-component"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Hi dpschen!
Thank you for creating a PR!
I've deployed the changes of this PR on a preview environment under this URL: https://1123-featurefeat-BaseButton-component--vikunja-frontend-preview.netlify.app
You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.
Have a nice day!
47441a55bb
toc526d90256
WIP: feat: create BaseButton componentto feat: create BaseButton component1327172494
tod2cdb30519
d2cdb30519
to40c75e92fd
a47cd0d0c9
toe242ef38a3
@ -0,0 +75,4 @@
// NOTE: we do not use scoped styles to reduce specifity and make it easy to overwrite
// We reset the default styles of a button element to enable easier styling
:where(.base-button--type-button) {
Why the
:where()
instead of just using.base-button--type-button
directly?By using
:where
as the main element class we make it possible to easily pverwrite the seit properties by a parent element. The reason beinh that the specifity of a class wrapped in:where
is lower than a class selector in the parent. This way we have sane default styles that are still easy to overwrite (aka without stuff like id selectors, duplicate classes, element selectors or even!important
.@ -15,3 +15,4 @@
<footer class="modal-card-foot is-flex is-justify-content-flex-end">
<x-button
v-if="tertiary !== ''"
:shadow="false"
Isn't this already implicit because of the variant
tertary
?Seems like it. Will remove :)
Btw it's tertIary. => fixed the spelling
Thanks :) I think I got that wrong in a few places...
@ -6,3 +4,1 @@
RENDER_FUNCTION: false,
WATCH_ARRAY: false, // TODO: check this again; this might lead to some problemes
TRANSITION_GROUP_ROOT: false,
MODE: 3,
Is it still required to configure this here even though it is already set in vite's config?
Would make sense.
I followed this guide (see step 3 under vite).
And then in the same guide under Global Confuguration they tell you to use this ConfuigureCompat method. This is why I made that error.
I guess the property in the vite config is alternative.
The location in vite.config seems better fitted since it's build related, so will remove here :)
So this essentially fixes the
a
/button
distinction?Yes! It abstracts functionality from style :)
I saw that there are many places in vikunja where link elements are used without a href attribute (which is invalid) and instead an
@click
listener e.g. to push a route. When pushing routes we should use router-link or maybe later some abstraction if that because it provides optimizations (e.g. the nuxt-link component dynamically preloads linked routes when the element gets in the viewport).Then there are places where spans / divs are used for click actions. This is not only semantically weong but also has some side effects that are unwanted (e.g. I thinl you have to add the prevent modifier which is not necessary with a native button, that's for free in the platform ™️ ). Where I realized that we need this functionality was when checking the password type toggle buttons: when you click them fast you will select the text in the input area behind. I'm not even sure how that could be prevented in js (it's not
user-select: none;
I think I'll add such a description in the element to make it clearer :)
Yes, that definitely is something that we should change.
f54ac7d062
tof6351e08ab
@konrad:
Can you check this again?
@ -0,0 +34,4 @@
submit: 'submit',
})
type BaseButtonTypes = keyof typeof BASE_BUTTON_TYPES_MAP
Why not a typescript string enum?
For this case: might make sense!
Why I still think we should keep it here:
In other components we might have a similar pattern where the values of the map are not strings but objects. E.g. think of a component that changes a configuration based on a prop where you just name the id of the configuration.
AFAIK that would be not possible with enums
Couldn't you use interfaces with typescript enums for that use case?
Not sure how you mean that?
An interface would just define the steucture of the configuration object, right?
So by doing this we would not recieve a configuration yet. What I want is the possibilty to chose between predefined configurations via a prop that provides an id (that selects one configuration).
I had something like this in mind:
That does not work though, so looks like I was wrong. I took a look in the docs and it seems like you can only use numeric or string literals in enums.
Anyway, as you correctly pointed out that would not make sense to do what I had in mind. I think your use case is completely valid here :)
For consistency between the button and base button I think it should be fine to keep this here.
f6351e08ab
toca51e5b348
ca51e5b348
toa0d91a93b1
a0d91a93b1
toccd1857867