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

style: Ensure minimum width for ArrivalByTimeChart #433

Merged
merged 4 commits into from
Jan 28, 2024
Merged

style: Ensure minimum width for ArrivalByTimeChart #433

merged 4 commits into from
Jan 28, 2024

Conversation

R894
Copy link
Collaborator

@R894 R894 commented Jan 24, 2024

Description

Fixes ArrivalByTimeChart being too small on lower resolutions by adding a minimum width to the component

Screenshots

Before:
image

After:
image

@R894 R894 requested a review from NoamGaash as a code owner January 24, 2024 17:09
@R894 R894 requested a review from shootermv January 24, 2024 17:15
Copy link
Contributor

@shootermv shootermv left a comment

Choose a reason for hiding this comment

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

nice catch!

@@ -51,7 +51,7 @@ export default function ArrivalByTimeChart({
.map((group) => (
<div key={group[0].name}>
<h3 className="title">{group[0].name}</h3>
<ResponsiveContainer debounce={1000} width="100%" height={300}>
<ResponsiveContainer debounce={1000} width="100%" minWidth={300} height={300}>
Copy link
Member

Choose a reason for hiding this comment

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

what's if the screen is narrower, or wider? I'm just not sure minWidth is the right solution for all devices

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

using negative left margin on the component seems to have a similar effect, what do you think?
image

Copy link
Member

Choose a reason for hiding this comment

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

negative margin is also a bad practice
I think this thread worth investigation - it seems like a very similar problem

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've looked into it and after experimenting the solution that worked for me is updating the package and setting a right margin of 35, but I'm still not sure if that's the best solution

Copy link
Member

Choose a reason for hiding this comment

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

That's far better! Thank you for doing this research

@NoamGaash NoamGaash changed the title fix: Ensure minimum width for ArrivalByTimeChart style: Ensure minimum width for ArrivalByTimeChart Jan 24, 2024
Copy link
Member

@NoamGaash NoamGaash left a comment

Choose a reason for hiding this comment

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

Cool!

@NoamGaash NoamGaash merged commit 971a6a0 into hasadna:main Jan 28, 2024
17 checks passed
VSciFlight added a commit to VSciFlight/open-bus-map-search that referenced this pull request Jan 30, 2024


* 'main' of https://github.com/VSciFlight/open-bus-map-search: (94 commits)
  chore: lint css (hasadna#442)
  style: Ensure minimum width for ArrivalByTimeChart  (hasadna#433)
  fix: Remove unknown data (hasadna#438)
  chore: Revert "chore: add temporary route for the version release event (hasadna#382)" (hasadna#441)
  ci: upgrade linter (hasadna#439)
  fix: scrollbar color on dark mode (hasadna#431)
  fix: Fix for issue hasadna#428 unable to run project locally (hasadna#430)
  chore: Adding tnufa to dropdown in dashboard hasadna#423 (hasadna#429)
  chore(main): release 1.0.0 (hasadna#326)
  fix: make input labels visible at dark mode (hasadna#424)
  fix: make top input label visible at singleLine page (hasadna#420)
  chore: add i18n type safety (hasadna#419)
  feat: add home-page files and temp route (hasadna#403)
  feat: finish bug report form creation (hasadna#411)
  feat: Translation of the remaining untranslated text on the About page (hasadna#399)
  test: add SinglelinePage tests (hasadna#393)
  feat: made logo white on dark mode (hasadna#397)
  feat: date picker component show invalid dates warning (hasadna#398)
  chore: hide bug report form (hasadna#388)
  docs: fixed a dead link in README.md (hasadna#396)
  ...
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.

3 participants