-
Notifications
You must be signed in to change notification settings - Fork 31
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
Mismatch version modal and toast #120
Conversation
✅ Deploy Preview for lodestone-dashboard ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Please manually link the issue using the side bar. |
const [showMajorVersionModal, setShowMajorVersionModal] = useState(false); | ||
const [dashboardVersion, setDashboardVersion] = useState(packageinfo.version); | ||
const [coreVersion, setCoreVersion] = useState(''); | ||
const openMajorVersionModal = ( |
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.
why is this named "open..."
it's a little confusing between openMajorVersionModal
and showMajorVersionModal
maybe just versionMismatchModal
?
); | ||
|
||
useEffect(() => { | ||
const endpoint = `http://localhost:${LODESTONE_PORT}/api/v1/info`; |
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.
axios has http://something: port/api/v1
set as the base url already, so you only need the /info
part
Also, consider using the useCoreInfo
hook
axios.get(endpoint).then((result) => { | ||
if (!result.data) return; | ||
setCoreVersion(result.data.version); | ||
const tempDashboardVersion = dashboardVersion.split('.'); |
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.
consider using a semver library to parse the version instead https://www.npmjs.com/package/semver
if (tempDashboardVersion[0] !== tempCoreVersion[0]) { | ||
setShowMajorVersionModal(true); | ||
} else { | ||
toast.warn('There is a minor/patch version mismatch!'); |
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.
maybe make the toast more specific. is it a minor or patch version?
what's the version of the dashboard, what's the version of the core.
Issue 23
Added a useEffect such that when the dashboard loads, there will be a comparison between the core and dashboard versions. A major version difference will cause a modal to pop up, otherwise, a toast will pop up.