Backup page #26

riffca · 2018-02-13T15:53:39Z

Добавлен компонент backup

roma219

В целом гуд, нужно немного подпилить. Чуть позже поподробнее посмотрю по функционалу компонент.

roma219 · 2018-02-13T16:00:27Z

client/root.js

@@ -10,7 +10,7 @@ export default {
  },
  render() {
    // eslint-disable-next-line
-    let app = (this.connected) ? (<router-view></router-view>) : (<h4>Connecting</h4>);
+    let app = (this.connected) || true ? (<router-view></router-view>) : (<h4>Connecting</h4>);


Я понимаю, что ты это ставишь, чтобы верстать удобнее было, не дожидаясь загрузки, но это изменение ломает другие компоненты, не забывай плз перед сабмитом PR убирать его.

у меня был коммит с более удобным управлением этим условием но он куда-то пропал в мержах

roma219 · 2018-02-13T16:01:24Z

client/store/app.js

+  setModal({ commit }, val) {
+    commit(APP_SET_MODAL, val);
+  },
+  setHeaderTitle({ commit }, val) {


Это не обязательно в стор выносить. Тебе ведь это нужно только в компоненте хедера? Можно обойтись простым computed там.

там была идея вначале менять в компоненте, потом в итоге вышло, что особо и нужно было. Но с другой стороны ничего плохого нет, что через стор хедер работает.

Да, работает, но совершаются лишние манипуляции, которые вообще без надобности (действия со стором и вотч), прошлый вариант с computed был проще и всё полностью выполнял. Верни, пожалуйста, к нему.

Вполне вероятна ситуация появления в нем надобности. Я же его и затеял, чтобы менять хедер в любом месте приложения при случае. Но отказался от реализации в текущем компоненте.

А зачем менять хедер в каком-то месте, если он отражает текущее местоположение по роуту? Если возникнет необходимость показать там какие-то еще данные, то для этого лучше будет использовать отдельный параметр, который отдельными экшенами можно будет устанавливать, который не будет меняться на вотч роута.

roma219 · 2018-02-13T16:01:41Z

client/store/app.js

+const APP_SET_MODAL = 'APP_SET_MODAL';
+
+const actions = {
+  setModal({ commit }, val) {


Что это за действие?

Это действие дает возможность использовать v-if для показа модального окна в зависимости от его названия.

Ок. Только давай будем использовать более понятные имена переменных. Если val - это название модального окна, то стоит обозначить его modalName.

roma219 · 2018-02-13T16:03:20Z

client/views/Backup/done.vue

+<script>
+
+import icon from '@/components/icon';
+/*eslint-disable*/


Убирай пожалуйста все выключения еслинта перед сабмитом PR

roma219 · 2018-02-13T16:04:42Z

client/views/Backup/done.vue

+
+import icon from '@/components/icon';
+/*eslint-disable*/
+const infos = [


А зачем это в отдельную переменную? В целом можно, но тогда стоит хранить просто в дате (а лучше - в $options, если это реактивные данные). А вообще хтмл код должен быть только в темплейте.

намного удобнее выносить списки текстов в переменные, нежели писать эти строки в html.

roma219 · 2018-02-13T16:06:07Z

client/views/Backup/index.vue

+
+#trusty_backup.main_padding
+
+	first(v-if="$route.name==='backup'")


Эти проверки можно тоже через компутед возвращать по словарю аналогично как в хедере было сделано.

roma219 · 2018-02-13T16:08:15Z

.editorconfig

@@ -1,7 +1,7 @@
 root = true

 [*]
-indent_style = space
+indent_style = tab


Что это за файл? Конфиг еслинта или?

извини,но ответ на это вопрос можно получить из названия файла..
я менял это потому что не очень ясно работает саблайм, webpack, eslinter и editor config вместе создавая дополнительных ошибки.

Ок, я просто не понял, что это за эдитор такой...

roma219 · 2018-02-13T18:08:23Z

.editorconfig

@@ -1,7 +1,7 @@
 root = true

 [*]
-indent_style = space
+indent_style = tab


Ок, я просто не понял, что это за эдитор такой...

roma219 · 2018-02-13T18:09:08Z

client/components/header/index.vue

    isProfilePage() {
      return this.$route.path.indexOf('home') !== -1 || this.$route.name === 'home';
-    },
-    getTitle() {
-      return this.titles[this.$route.name];


Вот этот вариант был лучше.

roma219 · 2018-02-13T18:10:45Z

client/style/pixel-glass.scss

@@ -10,7 +10,20 @@ HTML {
 /* Планшет */
 @media ( max-width: 768px ) {
  HTML {
-    background-image: url("./screens/manage_portfolio_manual.png");


pixel-glass - это для пиксель-перфект верстки ты используешь?

pixel glass да.. вещь)

roma219 · 2018-02-13T18:24:00Z

roma219

Ознакомься пожалуйста со стайл-гайдом по vue
https://vuejs.org/v2/style-guide/

roma219 · 2018-02-13T18:26:37Z

client/views/Backup/index.vue

+		p.trusty_help_text Let't verify your backup phrase
+		.verify_area
+			p.trusty_help_text
+				span(v-for="word in comprehendPhrase") {{ `${word} `}}


Можно же просто {{ word }}

roma219 · 2018-02-13T18:28:16Z

client/views/Backup/index.vue

+  },
+  computed: {
+    ...mapGetters('app', ['appModal']),
+    getPhrase() {


Здесь не нужен computed, это статические данные. На будущее, computed - это как бы как переменные, то есть существительные, а не вызываемые функции - лучше назвать просто phrase.

roma219 · 2018-02-13T18:32:03Z

client/views/Backup/index.vue

+		.modal_content: p.trusty_big_font TRY AGAIN
+
+	.modal_alert.main_padding(@click="setModal(null)", v-if="appModal==='backup_copied_password'")
+		.modal_content: p.trusty_big_font Password copied#[br] to clipboard


Можно сделать универсальный ui-компонент modal, который через slot будет получать, что ему отображать. По props получать состояние отобразается/спрятано. И его верстку не придётся в будущем по разным местам копировать.

roma219 · 2018-02-13T18:33:32Z

client/views/Backup/index.vue

+import { mapActions, mapGetters } from 'vuex';
+import phrase from './phrase';
+import first from './first';
+import done from './done';


https://vuejs.org/v2/style-guide/#Tightly-coupled-component-names-strongly-recommended

roma219 · 2018-02-13T18:35:13Z

client/store/app.js

+  }
+};
+
+const getters = {


Переименуй плз в getModalName, геттер - это функция, т.е. глагол и начинаем с get.

roma219 · 2018-02-15T03:50:28Z

client/views/Backup/index.vue

+	phrase(v-if="$route.name==='backup-phrase'", :phrase="getPhrase" )
+	done(v-if="$route.name==='backup-done'")
+
+	#verify(v-if="$route.name==='backup-verify'")


Вся эта затея с этими роутами делается через вложенные роуты - https://router.vuejs.org/en/essentials/nested-routes.html, и не придётся кучу этих проверок на роут.нейм писать.

youaresofunny · 2018-02-15T12:16:52Z

client/views/Backup/done.vue

+  `
+		I understand that my funds are<br/>
+		help securely on this device, not<br/>
+		by a compony, and nobody can<br/>


Опечатка в company

roma219

backup/verify

Некорректная работа при выборе всех слов. Надо, чтобы контейнер с выбранными словами имел скролл, когда слова больше по высоте. Также надо иметь возможность возвращать слова из выбранных в пул всех (обратное действие для выбора слова). Пофиксить ошибки, возникающие при выборе всех слов. На "clear" - возврат в исходное состояние. Сделать, чтобы кнопки-слова имели cursor: pointer.
route
Надо, чтобы по роуту backup/ грузился роут, который backup/info, для этого надо поменять url child-роута с "/info" на "/".
Выпилить из стора неиспользуемые вещи, связанные с headerTitle.

Когда завершишь, убери префикс wip: из PR - так будет ясно, что это уже готовый вариант, который можно смотреть.

riffca · 2018-02-19T03:42:43Z

Поправил по замечаниям. Хочу отметить что то пространство с выбранными кнопками скролится.

roma219 · 2018-02-19T10:15:54Z

@riffca Всё гуд, осталось совсем чутка:

По скроллу вижу, скроллится, просто есть какой-то странный скролл всего экрана... Ну это тогда отдельным issue.

riffca added 2 commits February 13, 2018 05:23

add makets and store app

f7a51af

add mobile backup page

ea40f1d

roma219 requested changes Feb 13, 2018

View reviewed changes

remove true

e957a02

roma219 requested changes Feb 13, 2018

View reviewed changes

riffca added 3 commits February 14, 2018 18:28

change icons and store args names

f9a8f08

fix prop

21388ab

add buttons in verify

4d1ea27

roma219 reviewed Feb 15, 2018

View reviewed changes

youaresofunny reviewed Feb 15, 2018

View reviewed changes

roma219 mentioned this pull request Feb 15, 2018

Доработка компоненты Backup #27

Closed

riffca added 3 commits February 16, 2018 10:47

change backup view

95f8dd8

merge with master

bdf28b0

hot changes

5098880

roma219 changed the title ~~Backup page~~ wip: Backup page Feb 16, 2018

rename

981a793

roma219 requested changes Feb 17, 2018

View reviewed changes

remove store header, fix backup verify phrase

6a6c981

riffca changed the title ~~wip: Backup page~~ Backup page Feb 19, 2018

fix route warn, fix getter, add spaces

caee391

roma219 merged commit fbc4299 into master Feb 20, 2018

roma219 deleted the backupPage branch February 20, 2018 03:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backup page #26

Backup page #26

riffca commented Feb 13, 2018

roma219 left a comment

roma219 Feb 13, 2018

riffca Feb 13, 2018

roma219 Feb 13, 2018

riffca Feb 13, 2018

roma219 Feb 13, 2018

riffca Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

riffca Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

riffca Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

riffca Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

riffca Feb 14, 2018

roma219 commented Feb 13, 2018

roma219 left a comment

roma219 Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 13, 2018

roma219 Feb 15, 2018

youaresofunny Feb 15, 2018

roma219 left a comment •

edited

Loading

riffca commented Feb 19, 2018

roma219 commented Feb 19, 2018


		#trusty_backup.main_padding

		first(v-if="$route.name==='backup'")

Backup page #26

Backup page #26

Conversation

riffca commented Feb 13, 2018

roma219 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

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

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

roma219 commented Feb 13, 2018

roma219 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

roma219 left a comment • edited Loading

Choose a reason for hiding this comment

riffca commented Feb 19, 2018

roma219 commented Feb 19, 2018

roma219 left a comment •

edited

Loading