New PR for margins in stable - fixes #9651 #9982

seekheart · 2018-02-10T18:13:36Z

new pr for #9903

Changes

see #9903 screenshots

UUID: 0848ed18-0966-4d90-83da-a1b4ae1500cc

Fixes #9651

negue

Thanks for your PR 👍

Could you please fix the mentioned fixes and revert the changes on:

website/static/presskit/Habitica - Gamify Your Life.mp4
website/static/presskit/presskit.zip

negue · 2018-02-10T19:31:03Z

website/client/components/inventory/item.vue

@@ -22,6 +22,9 @@ div(v-else)
    slot(name="popoverContent", :item="item")
 </template>

+<style>


could you remove the empty style tag?

negue · 2018-02-10T19:32:38Z

website/client/components/inventory/stable/petItem.vue

@@ -3,7 +3,7 @@ div
  .item-wrapper(@click="click()", :id="itemId")
    .item(
      :class="{'item-empty': emptyItem, 'highlight': highlightBorder}",
-    )
+    ).pet-slot


.pet-slot should be after .item before the attributes

it already is?

can you give me a snippet for clarity please?

sorry what I meant was .item.pet-slot(

alrighty, I couldn't git revert the two files you requested, is there an easier way?

also for the stuff under .pet-slot lines 7 I left them be.

seekheart · 2018-02-10T19:43:38Z

@negue how do I revert the changes on those files?

negue · 2018-02-10T20:03:15Z

to revert you can use this

git checkout origin/develop -- PATH/FILE for both mentioned files and then just commit, then those files will have the same content again ^^

seekheart · 2018-02-10T20:04:08Z

does that still apply if I forked the repo?

negue · 2018-02-10T20:06:01Z

yes it should work too, since HabitRPG/habitica should've been added as remote named origin and origin/develop points to our develop branch

seekheart · 2018-02-10T20:06:49Z

according to the wiki it was named upstream? so still origin or upstream?

negue · 2018-02-10T20:08:12Z

could you check how your remotes are named? mine still is at origin , maybe I added it myself

seekheart · 2018-02-10T20:16:00Z

it's upstream for me, I can't push the changes in till tmr anywho. I think I nailed everything you wanted. :)

negue · 2018-02-10T20:17:28Z

ok thank you 👍

seekheart · 2018-02-10T20:27:22Z

sorry to be a bother @negue I think I just successfully pushed the changes here :)

negue · 2018-02-10T20:30:07Z

@seekheart yes but now we also need to revert the saving pngs commit, I'd suggest to always use a different branch for fixes / features, not to push it always to develop :)

This reverts commit a38ea66.

seekheart · 2018-02-10T20:33:52Z

all set, thanks for the advice @negue hopefully this is the last thing I need to push xD.... in the future I'll use a feature branch

negue · 2018-02-10T20:36:55Z

it seems the files are still changed :/ maybe its just easier to create a third PR with only that css change and nothing else. and make you sure branch from upstream/develop not develop(of your local branch) that way you always have the latest changes

Alys · 2018-02-10T20:38:13Z

@seekheart Can you remove the mp4 and zip file too? They're likely to have been modified in your local install due to a problem that's unrelated to your changes, but you can avoid having irrelevant files included in a PR by committing only the files you changed. E.g., git commit filea fileb rather than git commit -a.

If you're not sure how to remove them, the info at the first couple of links from this search should help.

…develop

seekheart · 2018-02-10T20:49:41Z

I think I got it now @Alys the linked help and the two files are gone. @negue only css changes now :D

negue · 2018-02-11T16:29:13Z

website/client/components/inventory/stable/petItem.vue

  .pet-progress-bar {
    height: 4px;
    background-color: #24cc8f;
  }
+


could you please remove the duplicate css rule?

duplicate css rule? @negue

nvm I took care of it ;) @negue

Alys · 2018-02-12T11:12:47Z

@seekheart Can you also please do the Mounts? The issue mentions them as well and both should be modified together so it doesn't look inconsistent.

seekheart · 2018-02-12T14:06:17Z

@Alys are CSS classes global if I don't have <style scope> declaration? I figured I can just add .pet-slot to mountItem.vue?

paglias · 2018-02-12T15:34:34Z

Yes, anything not in a <style scoped> slot is global, but in that case for global css it's better to use the assets/scss folder

seekheart · 2018-02-13T00:22:54Z

@Alys @negue pushed the changes to mounts as well.

paglias · 2018-02-14T11:37:55Z

website/client/components/inventory/stable/mountItem.vue

@@ -15,6 +15,11 @@ div
    slot(name="popoverContent", :item="item")
 </template>

+<style lang="scss">


since this style block is not scoped can you move it to the assets/scss folder?

It's also a duplicate of the other change you made so you should just keep one version and move it to assets/scss

when I move to assets/scss is that going to be a new file and if so how do I call it? Is it called automatically from some build task?

I would put it into the existing item.scss file

seekheart · 2018-02-15T02:46:38Z

@paglias @negue it's now in one place under item.scss in assets.

paglias · 2018-02-15T12:11:20Z

website/client/assets/scss/item.scss

@@ -19,6 +19,10 @@
  margin-bottom: 8px;
 }

+.pet-slot {


just one small change and this should be good to go, can you change the selector from .pet-slot to .item.pet-slot so it's more specific?

paglias · 2018-02-19T17:55:11Z

@seekheart thanks for this! I'm merging the PR and awarding your contributor tier.

Note that further tiers require increasing amounts of work from one to the next, so Tier 2 may require a couple of PRs or a larger PR to attain. But keep helping out and we'll express our gratitude accordingly!

seekheart · 2018-02-19T17:59:27Z

@paglias it was fun contributing :)

paglias · 2018-02-19T18:01:46Z

@seekheart great! Sorry for the time it took to merge this in :)

seekheart added 6 commits January 28, 2018 16:05

made style change for pet item class to have more margins.

8d82383

made style change for pet item class to have more margins.

186d8bb

Merge branch 'develop' of github.com:seekheart/habitica into develop

d34cc02

files got added

a407a2b

encapsulated the items as stated in #9903

81ea2b9

#9903 moved the css to stable -> petitem.vue for better encapsulation.

c59760d

negue self-assigned this Feb 10, 2018

negue requested changes Feb 10, 2018

View reviewed changes

negue added the status: pr: with author: needs work label Feb 10, 2018

saving pngs

a38ea66

seekheart added 2 commits February 10, 2018 15:25

update on files

594236d

updated files as requested in PR!

61ce286

Revert "saving pngs"

e70e915

This reverts commit a38ea66.

Alys changed the title ~~New PR for margins in stable.~~ New PR for margins in stable - fixes #9651 Feb 10, 2018

seekheart added 2 commits February 10, 2018 15:42

made css changes just to pet slot

72d76a8

Merge branch 'develop' of https://github.com/seekheart/habitica into …

f65a538

…develop

negue reviewed Feb 11, 2018

View reviewed changes

#9982 removed redundant css

1e3ef50

#9982 applied scss to mount slots as well.

7889429

paglias suggested changes Feb 14, 2018

View reviewed changes

#9982 refactored scss to be in one place.

47935de

paglias added status: pr: ready for review and removed status: pr: with author: needs work labels Feb 15, 2018

paglias suggested changes Feb 15, 2018

View reviewed changes

paglias added status: pr: with author: needs work and removed status: pr: ready for review labels Feb 15, 2018

#9982 changed selector

98c3f20

paglias added status: pr: on Staging and removed status: pr: with author: needs work labels Feb 15, 2018

paglias merged commit 119f21d into HabitRPG:develop Feb 19, 2018

paglias added status: pr: merged in develop; deploy pending and removed status: pr: on Staging labels Feb 19, 2018

SabreCat removed the status: pr: merged in develop; deploy pending label Feb 23, 2018

snyk-bot mentioned this pull request Mar 25, 2021

[Snyk] Fix for 2 vulnerabilities Ivajkin/habitrpg#31

Open

New PR for margins in stable - fixes #9651 #9982

New PR for margins in stable - fixes #9651 #9982

Conversation

seekheart commented Feb 10, 2018 • edited by paglias Loading

Changes

negue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seekheart commented Feb 10, 2018

negue commented Feb 10, 2018

seekheart commented Feb 10, 2018

negue commented Feb 10, 2018 • edited Loading

seekheart commented Feb 10, 2018

negue commented Feb 10, 2018

seekheart commented Feb 10, 2018

negue commented Feb 10, 2018

seekheart commented Feb 10, 2018 • edited Loading

negue commented Feb 10, 2018

seekheart commented Feb 10, 2018

negue commented Feb 10, 2018

Alys commented Feb 10, 2018

seekheart commented Feb 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Alys commented Feb 12, 2018

seekheart commented Feb 12, 2018

paglias commented Feb 12, 2018

seekheart commented Feb 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seekheart commented Feb 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paglias commented Feb 19, 2018

seekheart commented Feb 19, 2018

paglias commented Feb 19, 2018

seekheart commented Feb 10, 2018 •

edited by paglias

Loading

negue commented Feb 10, 2018 •

edited

Loading

seekheart commented Feb 10, 2018 •

edited

Loading