-
Notifications
You must be signed in to change notification settings - Fork 17
Vite migration + added header #74
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Аня, дякую дуже за апдейтики)
глянула, полишала коменти невеличкі
ще які думки загальні:
- ми оновили версії react та vite, мінімальна необхідна версія ноди після цього мала піднятися — треба б чекнути і оновити версії в рідмішках в пререквізітах
- іноді форматування коду по файлах трохи неконсістент — хотіла запропонувати пройтися якимось prettier-ом, щоб усе однаково поформатовано було)
"version": "0.0.0", | ||
"type": "module", | ||
"scripts": { | ||
"dev": "vite", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add the --open
flag for automatic browser launch
"react-dom": "^16.14.0", | ||
"react-scripts": "^1.1.5", | ||
"@webdatarocks/react-webdatarocks": "^1.4.10" | ||
"@webdatarocks/react-webdatarocks": "^1.4.10", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest replacing this with "latest"
@@ -1,19 +1,28 @@ | |||
{ | |||
"name": "16.3", | |||
"version": "0.1.0", | |||
"name": "es6", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
може якесь інше ім'я? es6
якось наче надто загально)
background: #fff; | ||
position: relative; | ||
display: flex; | ||
border: 1px solid rgb(221, 221, 221); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
подумала що мені напевно без бордера все ж більше подобається демка, чистіше виглядає
але якщо хочеться, щоб тулбар не "висів", то може зробити такий бордер як у нас на демці, щоб бордер ніби як був частиною гріда і з'єднував тулбар з грідом?
@@ -0,0 +1,23 @@ | |||
import React, { Component } from 'react'; | |||
import './App.css'; | |||
import TopMenu from './components/TopMenu' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
додати ;
в кінці?
background: #fff; | ||
position: relative; | ||
display: flex; | ||
border: 1px solid rgb(221, 221, 221); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
те саме щодо бордера
import TopMenu from './components/TopMenu' | ||
import * as WebDataRocksReact from '@webdatarocks/react-webdatarocks'; | ||
|
||
class App extends Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
давай може вже на функціональні компоненти перепишем? там все майже те саме буде, просто це більш сучасний підхід для react apps, як я розумію
ref.current.webdatarocks.off("reportcomplete"); | ||
console.log(ref.current.webdatarocks); | ||
} | ||
class App extends Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
теж пропозиція переписати з функціональними компонентами
import ReactDOM from 'react-dom/client' | ||
import App from './App.tsx' | ||
import './App.css' | ||
import "@webdatarocks/webdatarocks/webdatarocks.min.css"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
стилі в css-ку?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
якось багато tsconfig
файлів, враження що можна якийсь прибрати
давай глянемо як в fm у нас?
No description provided.