Skip to content
Snippets Groups Projects

Address Issues #2434 And #3779 While Migrating Input Component from StyleGuide

Closed Hannes Heine requested to merge bgt-input-component-migration into master

Created by: Brandon-G-Tripp

:cake: Pullrequest

Issues

  • None
Screen Shot 2020-11-12 at 5 38 04 PM Screen Shot 2020-11-12 at 5 38 52 PM

Todo

  • Tests and Storybook Documentation

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
1 1 <template>
  • Created by: Mogge

    In general we prefer having small PRs addressing one issue. It looks like the changes here are to another issue. So please create another branch and another PR for this changes.

  • Created by: Brandon-G-Tripp

    Hey @Mogge, thank you for all of the really helpful feedback and the time for the suggestions. I would love to do a pairing session to work on the one small issue. I think it will help me understand the workflow for this project. Appreciate your time.

  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
  • 59 59 :onClick="commands.horizontal_rule"
  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
  • 12 12 </template>
    • Created by: Mogge

      I got a lot of warnings in the console: [Vue warn]: Invalid prop: type check failed for prop "name". Expected String with value "null", got Null

  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
  • 16 16 v-model="form.email"
    17 17 :disabled="pending"
    18 18 :placeholder="$t('login.email')"
    19 19 type="email"
    20 20 name="email"
    21 21 icon="envelope"
    22 22 />
    23 <ds-input
    23 <base-input
    24 24 v-model="form.password"
    25 25 :disabled="pending"
    26 26 :placeholder="$t('login.password')"
    27 27 icon="lock"
    28 icon-right="question-circle"
    28 icon-right="arrow-down"
    29 icon-right-secondary="eye"
    • Created by: Mogge

      As I stated in the related issue, I think it is no good idea to overload our input component with another icon. I prefer a warning message under the password field that pops up when capslock is active. This seems to be easier to realize and we can do it directly in the LoginForm component.

  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
  • 39 39 display: inline-flex;
    40 40 vertical-align: bottom;
    41 41
    42
  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
  • 10 iconRightSecondary && `input-has-icon-right-secondary`
    11 ]"
    12 :id="id"
    13 :name="name ? name : model"
    14 :type="[showPassword ? 'text' : 'password']"
    15 :v-model="type"
    16 :autofocus="autofocus"
    17 :placeholder="placeholder"
    18 :tabindex="tabindex"
    19 :disabled="disabled"
    20 :readonly="readonly"
    21 :value.prop="innerValue"
    22 @input="handleInput"
    23 @focus="handleFocus"
    24 @blur="handleBlur"
    25 v-on:keydown.caps-lock="caps = true"
  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
  • 18 18 :placeholder="$t('login.email')"
    19 19 type="email"
    20 20 name="email"
    21 21 icon="envelope"
    22 22 />
    23 <ds-input
    23 <base-input
    24 24 v-model="form.password"
    25 25 :disabled="pending"
    26 26 :placeholder="$t('login.password')"
    27 27 icon="lock"
    28 icon-right="question-circle"
    28 icon-right="arrow-down"
    29 icon-right-secondary="eye"
    29 30 name="password"
    30 31 type="password"
    • Created by: Mogge

      What about something easy like:

      :type="showPassword ? 'text' : 'password'"

      And change the icon the same way.

  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
  • 17 :placeholder="placeholder"
    18 :tabindex="tabindex"
    19 :disabled="disabled"
    20 :readonly="readonly"
    21 :value.prop="innerValue"
    22 @input="handleInput"
    23 @focus="handleFocus"
    24 @blur="handleBlur"
    25 v-on:keydown.caps-lock="caps = true"
    26 v-on:keyup.caps-lock="caps = false"
    27 />
    28 <base-icon
    29 class="input-icon-right"
    30 v-show="caps"
    31 :name="iconRight" />
    32 <a
  • Created by: Mogge

    Review: Changes requested

    I suggest to split this PR in at least three different PRs. The first should be the show password button, which I think is the easiest one. Next would be the capslock indicator and third the migration of ds-input. The help for the editor would be another one. If you want, we can have a pairing session.

  • Please register or sign in to reply
    Loading