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

Highcharts/3 stacked bar #4

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

MarkusBarstad
Copy link
Contributor

No description provided.

@MarkusBarstad MarkusBarstad self-assigned this Aug 22, 2022
highcharts-api/highcharts/3-stacked-bar/index.html Outdated Show resolved Hide resolved
highcharts-api/highcharts/3-stacked-bar/main.js Outdated Show resolved Hide resolved
highcharts-api/highcharts/3-stacked-bar/main.js Outdated Show resolved Hide resolved
highcharts-api/highcharts/3-stacked-bar/main.js Outdated Show resolved Hide resolved
highcharts-api/highcharts/3-stacked-bar/main.js Outdated Show resolved Hide resolved
@MarkusBarstad
Copy link
Contributor Author

Thank you for good feedback, I will adjust my answer now!

@MarkusBarstad
Copy link
Contributor Author

MarkusBarstad commented Aug 30, 2022

I tried to implement some of the changes you suggested and I also refactored a bit, and I do feel that it is working more appropriately now!

Also, I decided to ditch the lines around the categories since you said it was ok ;)

highcharts-api/highcharts/3-stacked-bar/main.js Outdated Show resolved Hide resolved
highcharts-api/highcharts/3-stacked-bar/main.js Outdated Show resolved Hide resolved
highcharts-api/highcharts/3-stacked-bar/main.js Outdated Show resolved Hide resolved
highcharts-api/highcharts/3-stacked-bar/main.js Outdated Show resolved Hide resolved
@MarkusBarstad
Copy link
Contributor Author

Hello Michał! I have tried to:

  • Use more button-properties as you suggested(i also noticed i had some needless duplicate code relating to buttons)
  • I have also adjusted margin to marginTop as you suggested
  • I also set the max-value as per your suggestion

Copy link

@mfilipiec mfilipiec left a comment

Choose a reason for hiding this comment

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

Good job! I have no more objections. You can mark the task as done. 🎉 😃

@MarkusBarstad
Copy link
Contributor Author

Thank you, Michał, this was a good exercise :D!

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.

2 participants