Skip to content

Tiny graphical edit on in instrument tab #13

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.

Sign up for GitHub

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

Closed
FredericGuilbault wants to merge 3 commits into from
Closed

Tiny graphical edit on in instrument tab #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fb34178
Tiny graphical edit on in instrument tab
FredericGuilbault Apr 2, 2018
29a29ea
Use a more precise base map for +185ko
FredericGuilbault Apr 3, 2018
b26d36e
Update README.md
FredericGuilbault Apr 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ Plotter accessible at http://localhost:4999/
To see actual data, you should have a [signalk-server-node](https://github.com/SignalK/signalk-server-node)
running and maybe some [charts](https://github.com/vokkim/tuktuk-chart-plotter#charts).

# Developement

Install dependencies:

`npm install`

Start server:

`npm run watch`

## Environment variables

- `PORT` = server port, default 4999
Expand Down Expand Up @@ -157,4 +167,4 @@ Other charts in `client-config.json` are of type `tilelayer`:

## License

MIT
MIT
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"eslint-plugin-es5": "1.2.0",
"eslint-plugin-react": "7.7.0",
"express": "4.16.3",
"@geo-maps/countries-coastline-10km": "0.6.0",

@vokkim vokkim Apr 10, 2018

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is the countries-coastline-10km package necessary if the actual JSON file has been copied under world-base.geo.json? Maybe we could just add credits to the readme.

@FredericGuilbault FredericGuilbault Apr 11, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I would go the other way.
For more fluid maintenance in time. The JSON should be copied automatically on build or used strait from it's original location.

Im not much used to nodejs but this seem the way to go. Although I woudn't know what would be the best strategy. So I did not push anything on this part exept adding the source package.

It's an improvement but an incomplete feature. Maybe it should come with the opening of an issue.

@FredericGuilbault FredericGuilbault Apr 17, 2018 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@vokkim What would be the best thing to do with this ?

Im keeping it in my fork untill it's a full feature.

You accept it like that and I open an issue about it on your repo.

Or I remove the dependency, add credits to the readme and you will accept it.

"geolib": "2.0.24",
"leaflet": "1.3.1",
"leaflet-rotatedmarker": "0.2.0",
Expand Down
239 changes: 238 additions & 1 deletion public/world-base.geo.json
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/styles/app.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,14 @@ p {
flex: 1 1 auto;
font-size: 110%;
color: @labelFontColor;
text-align: right;
text-align: center;

@vokkim vokkim Apr 10, 2018 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Setting text-align: center will make the text "jump" when text length/value changes. I'd still prefer aligning the text on right, but maybe adding some padding or margin to the right.

@FredericGuilbault FredericGuilbault Apr 11, 2018 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fair point.
See better approach on PR #14

}
.value {
.instruments-font();
margin: 10px 0;
font-size: 350%;
color: @lightFontColor;
text-align: right;
text-align: center;
}
}

Expand Down