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

Fix: timeline in programs page breaking #249

Closed
wants to merge 0 commits into from

Conversation

nlok5923
Copy link
Contributor

@nlok5923 nlok5923 commented Mar 6, 2021

Description

To resolve this issue currently i had changed the positioning value as previously the date[0][1] for both the programs are different which results in incorrect positioning of timeline now it has been resolved by consumig value date[2][1] as date[2][1] is nearly same for both the programs.

Fixes #246

Type of Change:

Delete irrelevant options.

  • Code
  • User Interface

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

deployment link: https://hardcore-booth-457055.netlify.app/

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have attached link of deployed site.

Code/Quality Assurance Only

  • My changes generate no new warnings

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 6, 2021

@iamkryptonite @nandini45 please review the solution i approached for resolving the issue.

@iamkryptonite
Copy link
Contributor

@nlok5923 the current date is still being shown in the wrong place. Also you can use gh-pages to deploy instead of heroku.

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 7, 2021

@iamkryptonite as today is 7th of march so it is showing:
image
which is quite near to marker for date 1 march which says today's date is 7th march.
image

please correct me @iamkryptonite if am missing something.

@iamkryptonite
Copy link
Contributor

@nlok5923 if you scroll at the very beginning you will see this.
AnitaB_org_open_source

7th of March(todays date) is being shown in the month of January. It should be in March. And according the Fade and Marker component will be adjusted.

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 7, 2021

@iamkryptonite please can you test the deployed site again i think the changes are addressed in latest commit:
image

@Rahulm2310
Copy link
Contributor

@nlok5923 the timeline is overlapping the program title
image
image
Please fix this.

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 7, 2021

Thanks @Rahulm2310 for pointing out the issue will resolve it in the latest commit.

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 8, 2021

@Rahulm2310 i think you were testing it at custom resolution as it will definitely throws some distortion in custom resolution as here positioning has been made using date value

@iamkryptonite can you suggest me some clue to bring dynamicity in positioning.

@iamkryptonite
Copy link
Contributor

For now maybe keep the Events component fixed at the very beginning and adjust the other components so that they don't overlap in smaller resolutions. Use both date and month to adjust the positions like its done in the Date component and tweak it a little bit. Hope this helps.
AnitaB_org_open_source

Also the line component should start from the first given date for each individual event.

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 8, 2021

changes in latest commit:

  • Event components now have fix position.
  • line start from start date of year
    image
    image

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 8, 2021

@iamkryptonite please can you review the changes

@iamkryptonite
Copy link
Contributor

@nlok5923 Looks good just a few small changes:

  1. For smaller screens the "events" are still overlapping with the dates. To test this go to inspect elements, then toggle device toolbar(phone/tablet icon) and select Laptop-1024px.
  2. When the component is loaded try to keep the current date in the middle.
  3. Add some padding between the marker and the current date .
  4. Add a few more dummy dates if possible.

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 9, 2021

@iamkryptonite the dates for events in timelines are actual ones i think we can't put dummy dates there. Also i still doubt how can we stop overlapping of event components with date in smaller resolution. the event component has fixed size so do i need to alter the size of event component or i need to do something else to make it dynamic .

rest points will get address in latest commit.

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 9, 2021

changes in latest commit:
image

@nlok5923
Copy link
Contributor Author

nlok5923 commented Mar 9, 2021

removed overlapping at smaller resolutions .
image

test deployment here: https://hardcore-booth-457055.netlify.app/

@iamkryptonite
Copy link
Contributor

Looks good!

@nlok5923
Copy link
Contributor Author

@annabauza can u please review this pull request.

@annabauza
Copy link
Contributor

@annabauza can u please review this pull request.

Approved require one more review

@isabelcosta
Copy link
Member

I don't quite understand the change here. I don't understand the diagram 😳 @iamkryptonite if you approve this if you think it looks good I am happy to merge this. We just need a second approval

@isabelcosta isabelcosta added the Status: Needs Review PR needs an additional review or a maintainer's review. label Oct 8, 2021
@isabelcosta
Copy link
Member

@nlok5923 could you fix the merge conflicts, please?

@isabelcosta isabelcosta added the Status: Changes Requested Changes are required to be done by the PR author. label Oct 17, 2021
@nlok5923 nlok5923 closed this Oct 18, 2021
@nlok5923
Copy link
Contributor Author

@isabelcosta i will open a new pr for this changes as i need to close this pr due to force push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Changes Requested Changes are required to be done by the PR author. Status: Needs Review PR needs an additional review or a maintainer's review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Timeline in 'Programs' page is breaking
5 participants