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

Add vim mode1 #433

Merged
merged 13 commits into from Jan 27, 2019
Merged

Add vim mode1 #433

merged 13 commits into from Jan 27, 2019

Conversation

skhrv
Copy link
Collaborator

@skhrv skhrv commented Jan 20, 2019

No description provided.

Copy link
Contributor

@imamatory imamatory left a comment

Choose a reason for hiding this comment

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

Режим редактора должен персиститься либо на сервер, либо в local storage. Фича не сильно важная поэтому в local storage лучше. Для этого надо сделать persisted reducer через redux-persist.
Могу помочь с этим

return (
<div style={{ position: 'relative' }}>
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Может этот div уже не нужен?

Copy link
Contributor

Choose a reason for hiding this comment

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

У нас уже есть бекенд код для персистинга.
Надо просто его завести)
Как и язык

this.vimMode = initVimMode(this.editor, this.statusBarRef.current);
} else if (!isVimMode && prevProps.isVimMode) {
this.vimMode.dispose();
this.statusBarRef.current.innerHTML = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Что такое statusBar? Почему ему надо обнулять html?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

показывает какой режим включен в виме
обнуление вообще делается внутри monaco-vim при отключение режима, но видимо не очень дружит он с реактом, поэтому пришлось вынести

@@ -98,19 +111,21 @@ class Editor extends PureComponent {
enabled: false,
},
};
const editorHeightWithVimMode = isVimMode ? editorHeight.replace(/[a-z]/g, '') - this.statusBarHeight : editorHeight;
Copy link
Contributor

Choose a reason for hiding this comment

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

Больно как-то регуляркой выбирать числа, может можно проще? Тут, конечно, глядеть надо, так непонятно под што подстраивается высота

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ага, можно) погорячился

@skhrv
Copy link
Collaborator Author

skhrv commented Jan 20, 2019

добавил redux-persist, крутая штука) не знал что так можно

import RootContainer from './containers/RootContainer';
import createStore from './lib/configureStore';
import reducers from './reducers';
import GameList from './containers/GameList';


const persistConfig = {
key: 'root',
Copy link
Contributor

Choose a reason for hiding this comment

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

Не ставь на root, ставь только на нужный редьюсер.
пример:

const { auth: authReducer, ...otherReducers } = reducers;

const authPersistConfig = {
  key: 'auth',
  storage,
};

const rootReducer = combineReducers({
  auth: persistReducer(authPersistConfig, authReducer),
  ...otherReducers,
});

@@ -159,3 +159,5 @@ export const currentChatUserSelector = (state) => {
const currentUser = _.find(chatUsersSelector(state), { id: currentUserId });
return currentUser;
};

export const editorVimModeSelector = userId => state => _.get(editorDataSelector(userId)(state), 'isVimMode', false);
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

там не очень то плоский редьюсер у редактора

const initialState = {
  meta: {},
  text: {},
};

не хочется там копаться и не известно же какие userId будут

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ок, понял, завтра сделаю

@vtm9
Copy link
Contributor

vtm9 commented Jan 21, 2019

добавьте еще емакс моде и выбор темы dark/light

@imamatory
Copy link
Contributor

Темы я бы в отдельном по сделал.

@vtm9
Copy link
Contributor

vtm9 commented Jan 21, 2019

Можно в отдельном ПО

@skhrv
Copy link
Collaborator Author

skhrv commented Jan 21, 2019

отдельное ПО?

@vtm9
Copy link
Contributor

vtm9 commented Jan 21, 2019

ПО === PR

@imamatory
Copy link
Contributor

@skhrv пингани, когда будет готово

@@ -29,3 +29,6 @@ export const updateCheckStatus = createAction('UPDATE_CHECK_STATUS');

export const compressEditorHeight = createAction('COMPRESS_EDITOR_HEIGHT');
export const expandEditorHeight = createAction('EXPAND_EDITOR_HEIGHT');

export const toggleVimMode = createAction('MODE_VIM_TOGGLE');
export const toggleDefaultMode = createAction('MODE_DEFAULT_TOGGLE');
Copy link
Contributor

Choose a reason for hiding this comment

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

Мне кажется лучше сделать один action с параметром режима, типа setEditorMode(mode: string)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

да, исправил

type="button"
className="dropdown-item btn rounded-0"
key={type}
onClick={() => action()}
Copy link
Contributor

Choose a reason for hiding this comment

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

onClick={() => action()} -> onClick={action}

imamatory
imamatory previously approved these changes Jan 27, 2019
@imamatory
Copy link
Contributor

Надо конфликты порешать

@imamatory imamatory merged commit c202c55 into hexlet-codebattle:master Jan 27, 2019
puku pushed a commit to puku/codebattle that referenced this pull request Jan 31, 2019
* add VimMode

* fix buttons in toolbar

* improve Editor.jsx

* add redux-persist

* add padding for vim statusBar

* fix persisted reducer

* add EditorsModeToggle

* add editorUI reducer

* refactor

* fix editorsHeight

* fix async load not included syntax for spectators

* refactor
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.

None yet

3 participants