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

Added click and drag #92

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Added click and drag #92

wants to merge 13 commits into from

Conversation

arik123
Copy link
Collaborator

@arik123 arik123 commented Nov 13, 2020

Added the option to click and drag while selecting multiple items.
#91

@ZeusJunior
Copy link
Owner

ZeusJunior commented Nov 16, 2020

b17c551 Shouldn't have to use marketplace for effects, we have them local: 20d2cad

Clicking to select doesn't seem to work anymore, and clicking to deselect either reselects it instantly or has a small chance to actually unselect it

@ZeusJunior
Copy link
Owner

Clicking to select doesn't seem to work anymore, and clicking to deselect either reselects it instantly or has a small chance to actually unselect it

On the grid view, not list view. That works nicely

@arik123
Copy link
Collaborator Author

arik123 commented Nov 16, 2020

b17c551 Shouldn't have to use marketplace for effects, we have them local: 20d2cad

this is handled in 2c2fb37

@arik123
Copy link
Collaborator Author

arik123 commented Nov 16, 2020

Clicking to select doesn't seem to work anymore, and clicking to deselect either reselects it instantly or has a small chance to actually unselect it

I've also noticed it, but I'm not sure how to fix it. It seems to be a problem with mousedown event not firing or propagating properly.

@ZeusJunior
Copy link
Owner

That should have fixed it

@arik123
Copy link
Collaborator Author

arik123 commented Nov 16, 2020

d939f41 havent fixed it, now it does not select the first item you have clicked.
select

'Apotheosis',
'Ascension',
'Reindoonicorn Rancher',
// 'Reindoonicorn Rancher',
Copy link

Choose a reason for hiding this comment

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

don't commit commented code

'5061;6': 'a89a8c', // Waterlogged Lab Coat
'5036;6': '7c6c57', // Ye Olde Rustic Colour
'5028;6': '424f3b' // Zepheniah's Greed
// A Color Similar to Slate
Copy link

Choose a reason for hiding this comment

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

Why did you decide to open new lines? In practice this saves about 2 characters of horizontal space

background-color: #666;
}
.item:hover .price {
visibility: visible;
}

#pricelist {
Copy link

Choose a reason for hiding this comment

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

Try not to target on an ID directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what is the problem with targeting ID ?

Copy link

Choose a reason for hiding this comment

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

https://stackoverflow.com/questions/8084555/why-selecting-by-id-is-not-recommended-in-css You're basically cornering yourself in, and won't be able to override it later with another selector.

@@ -61,6 +61,16 @@ const app = new Vue({
list: false
},
methods: {
mouseEntered: function (item, e) {
if (this.multiSelect.enabled && (e.buttons && e.buttons==1) && e.button == 0) {
Copy link

Choose a reason for hiding this comment

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

e.buttons == 1 instead of e.buttons==1 , seems not to match your codestyle

  • try not to hook everything in brackets, a general preferance in JS is
if (!this.multiSelect.enabled || !(e.buttons && e.buttons==1) || e.button !== 0)
    return;

And that way save on horizontal space

@@ -259,7 +259,9 @@
</div>

<div v-if="!list" class="d-flex mt-3 flex justify-content-center flex-wrap" id="pricelist">
<div class="item item-grid m-1" v-for="item in pricelistSorted" :key="item.sku" :style="{ backgroundImage: `url( ${item.style.image_small} ), url( ${item.style.effect} )`, backgroundColor: item.style.quality_color, borderStyle: item.style.craftable? false : 'dashed', opacity: item.enabled?1:0.5, borderColor: item.style.border_color}" @click="itemClick(item, $event)">
<div class="item item-grid m-1" v-for="item in pricelistSorted" :key="item.sku"
:style="{ backgroundImage: `url( ${item.style.image_small} ), url( ${item.style.effect} )`, backgroundColor: item.style.quality_color, borderStyle: item.style.craftable? false : 'dashed', opacity: item.enabled?1:0.5, borderColor: item.style.border_color}"
Copy link

@poespas poespas Feb 23, 2021

Choose a reason for hiding this comment

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

Please split the style-properties up on new lines

<div class="item item-grid m-1" v-for="item in pricelistSorted" :key="item.sku" :style="{ backgroundImage: `url( ${item.style.image_small} ), url( ${item.style.effect} )`, backgroundColor: item.style.quality_color, borderStyle: item.style.craftable? false : 'dashed', opacity: item.enabled?1:0.5, borderColor: item.style.border_color}" @click="itemClick(item, $event)">
<div class="item item-grid m-1" v-for="item in pricelistSorted" :key="item.sku"
:style="{ backgroundImage: `url( ${item.style.image_small} ), url( ${item.style.effect} )`, backgroundColor: item.style.quality_color, borderStyle: item.style.craftable? false : 'dashed', opacity: item.enabled?1:0.5, borderColor: item.style.border_color}"
@click="itemClick(item, $event)" @mouseenter="mouseEntered(item, $event)">
Copy link

Choose a reason for hiding this comment

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

either make the decision, in general HTML, to put attributes on all new lines, or put them all on the same line. this will avoid confusion.

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.

4 participants