Skip to content
This repository has been archived by the owner on Feb 8, 2019. It is now read-only.

Add basic UI components #10

Merged
merged 28 commits into from Aug 22, 2018
Merged

Add basic UI components #10

merged 28 commits into from Aug 22, 2018

Conversation

amaury1093
Copy link
Contributor

@amaury1093 amaury1093 commented Aug 19, 2018

Add some basic UI in this PR. Note: nothing is connected to the backend yet, there is a temporary mockData.ts file which holds some fake territories of one nation, which evolves over time.

  • Add a left pane which will hold nation information
  • Add a slider on the bottom to travel through time
  • Add the list of territories of this nation (from mock data)
  • Show these territories on the map, only show the territory that is current to the current date
  • Add a possibility to edit the current territory (only if you're connected). The "Save" button doesn't do anything yet.
  • If you change date while editing, the editing mode is turned off.

Edit: sorry about the git avatars not being there, i got a new computer, but forgot to do git config...

@vercel
Copy link

vercel bot commented Aug 19, 2018

This pull request is automatically deployed with Now.

To access deployments, click Details below or on the icon next to each push.

@amaury1093
Copy link
Contributor Author

Fixing Now deployment

Copy link
Contributor

@quorth0n quorth0n left a comment

Choose a reason for hiding this comment

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

Good stuff here, love the design choices. Unfortunately it doesn't seem to work on Firefox. All years show up as NaN, the territory polygons are not displayed, and clicking the edit button causes the map to re-render. and the edit tools disappear. I found the two following errors in console:

The connection to http://localhost:3000/_next/webpack-hmr was interrupted while the page was loading. main.js:35058:13
Warning: Text content did not match. Server: "395" Client: "NaN" warning.js:33
	printWarning warning.js:33
	warning warning.js:57
	warnForTextDifference react-dom.development.js:7235
	warnForUnmatchedText$1 react-dom.development.js:8008
	didNotMatchHydratedTextInstance react-dom.development.js:8702
	prepareToHydrateHostTextInstance react-dom.development.js:12925
	completeWork react-dom.development.js:14139
	completeUnitOfWork react-dom.development.js:15713
	performUnitOfWork react-dom.development.js:15890
	workLoop react-dom.development.js:15902
	renderRoot react-dom.development.js:15942
	performWorkOnRoot react-dom.development.js:16560
	performWork react-dom.development.js:16482
	performSyncWork react-dom.development.js:16454
	requestWork react-dom.development.js:16354
	scheduleWork$1 react-dom.development.js:16218
	scheduleRootUpdate react-dom.development.js:16785
	updateContainerAtExpirationTime react-dom.development.js:16812
	updateContainer react-dom.development.js:16839
	../../node_modules/react-dom/cjs/react-dom.development.js/ReactRoot.prototype.render react-dom.development.js:17122
	legacyRenderSubtreeIntoContainer/< react-dom.development.js:17262
	unbatchedUpdates react-dom.development.js:16679
	legacyRenderSubtreeIntoContainer react-dom.development.js:17258
	hydrate react-dom.development.js:17314
	renderReactElement index.js:532
	_callee5$ index.js:507
	tryCatch runtime.js:62
	invoke runtime.js:296
	defineIteratorMethods/</prototype[method] runtime.js:114
	step asyncToGenerator.js:12
	_next asyncToGenerator.js:27
	_asyncToGenerator/</< asyncToGenerator.js:34
	Promise es6.promise.js:177
	_asyncToGenerator/< asyncToGenerator.js:7
	_doRender index.js:524
	doRender index.js:406
	_callee2$ index.js:312
	tryCatch runtime.js:62
	invoke runtime.js:296
	defineIteratorMethods/</prototype[method] runtime.js:114
	step asyncToGenerator.js:12
	_next asyncToGenerator.js:27
	_asyncToGenerator/</< asyncToGenerator.js:34
	Promise es6.promise.js:177
	_asyncToGenerator/< asyncToGenerator.js:7
	_render index.js:342
	render index.js:284
	_callee$ index.js:263
	tryCatch runtime.js:62
	invoke runtime.js:296
	defineIteratorMethods/</prototype[method] runtime.js:114
	step asyncToGenerator.js:12
	_next asyncToGenerator.js:27
	run es6.promise.js:75
	notify/< es6.promise.js:92
	flush _microtask.js:18

On Chromium it's running fine for me however.

I also think that it might be worth pointing out that the site isn't mobile responsive, but that might be better to introduce in a separate PR.
image

@amaury1093
Copy link
Contributor Author

Yes, thanks for the comments.

  • http://localhost:3000/_next/webpack-hmr I have a suspicion on where this come from: We're using dynamic routes (e.g. /map/yyyy/mm/dd), but it requires a custom server script. I filed an issue here: Add dynamic routing vercel/next.js#4989. However the error doesn't appear on production, and HMR still works. So i'll leave this for now.
  • Fixed NaN and territories on firefox, it was due to a difference of the javascript Date object behaving differently.
  • Thanks for the design compliments. However, I'm not a designer, and that's the best I can do, and I actually feel that it could be way better. If at some point we could have an UI/UX designer in our team, that'd be excellent.
  • Agreed with responsiveness, will address in a separate PR.

const endDate = endDateFromData ? endDateFromData : new Date();

// Convert `startDate` to yyyy/mm/dd format
const url = startDate
Copy link
Contributor

Choose a reason for hiding this comment

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

I've noticed that when clicking the territory evolution date cards (with the strange exception of the earliest card), the date is changed to one year before the start date. Should one year be added to this value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

onChange={handleChangeYear}
railStyle={{ backgroundColor: 'grey', height: 10 }}
trackStyle={{ backgroundColor: '#4a88cb', height: 10 }}
value={currentDate.getFullYear()}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if a slider will be the best way to select dates in the future, a lot can happen in a single year. We should probably change the Current Year card to a date picker and maybe just leave the slider as a visual for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed with you, I removed the slider completely.

@amaury1093
Copy link
Contributor Author

I was looking at the alternatives of a React date picker component, but didn't find the perfect one:

I'm looking for one where it's easy to navigate between years.

  • react-dates by airbnb. It's super cool, but you can't change years easily. You can customize it, but there's some design work to do. Also, it depends on moment, which I want to avoid.
  • react-date-picker. To go way back in time, you have to choose your century, decade, year etc. Not the best UX.
  • react-datepicker: Doesn't go back to earlier than 1900 (Year dropdown defaults back to 1900 Hacker0x01/react-datepicker#893). Depends on moment.
  • HTML5 date input: Doesn't render the same across browsers

Didn't try others yet. I'll try some others later, maybe in another PR. If not, then I'll create a custom datepicker (probably will be based on airbnb's one).

For now, in the scope of this PR, I'll use the HTML5 one.

@quorth0n
Copy link
Contributor

Yeah I'd say react-dates with some customization is probably the best option. Have you seen react-dates/react-dates#1106? It looks like this recent PR might help us out a bit for our customization.
Why do you want to avoid moment.js?
Also, might be worth noting the design of the date picker in a game that a lot of our mappers play. It's unique but is pretty intuitive and makes it easy to select dates. The arrows above and below the year allow you to change by century/decade/year, a fourth for millennia might suit this project.
screenshot 2018-08-21 at 6 00 04 pm

@amaury1093
Copy link
Contributor Author

Yes, I saw that PR, that's what I meant by "some design work" in my previous message, we needed to design the month/year selector ourselves. But this comment react-dates/react-dates#1106 (comment) seems to be cool.

I really like the game one. react-dates + this customization would be really nice.

I want to avoid moment because of the bundle size: see moment/moment#3376 or tons of other articles on the net.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants