Skip to content
Snippets Groups Projects

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

Closed Address Issues #2434 And #3779 While Migrating Input Component from StyleGuide
bgt-input-component-migration into master
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
webapp/components/Editor/Editor.vue
1 1 <template>
  • Hannes Heine
    Hannes Heine @Elweyn ·
    Author Owner

    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.

  • Hannes Heine
    Hannes Heine @Elweyn ·
    Author Owner

    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
    • webapp/components/Editor/MenuBar.vue
    59 59 :onClick="commands.horizontal_rule"
  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
    • webapp/components/LoginForm/LoginForm.vue
    12 12 </template>
    • Hannes Heine
      Hannes Heine @Elweyn ·
      Author Owner

      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
    • webapp/components/LoginForm/LoginForm.vue
    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"
    • Hannes Heine
      Hannes Heine @Elweyn ·
      Author Owner

      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
    • webapp/components/_new/generic/BaseIcon/BaseIcon.vue
    39 39 display: inline-flex;
    40 40 vertical-align: bottom;
    41 41
    42
  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit acdd67d5
    • webapp/components/_new/generic/BaseInput/BaseInput.vue
    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
    • webapp/components/LoginForm/LoginForm.vue
    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"
    • Hannes Heine
      Hannes Heine @Elweyn ·
      Author Owner

      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
    • webapp/components/_new/generic/BaseInput/BaseInput.vue
    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
  • Hannes Heine
    Hannes Heine @Elweyn ·
    Author Owner

    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