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

Update Team Meeting Data on the Events page to Show Meeting Frequency #7043

Conversation

terrencejihoonjung
Copy link
Member

Fixes #6920

What changes did you make?

  1. Modified vrms_data.json to include "isWeekly": {boolean} key-value pair based on meeting description's content
  2. Conditionally rendering "*", which is the HTML entity for an asterik based on isWeekly property for each event.
  3. Mapping the isWeekly property within the display_object function to include this property for each event object
  4. Added appropriate text at the end of the "Online Project Team Meetings" section

Why did you make the changes (we will use this info to test)?

  1. To utilize the new property for conditional rendering of the asterik.
  2. To denote whether an event's meetings are weekly or not.
  3. To include the isWeekly property for each event object.
  4. To clarify what the asterik siginifies for users.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied Before >
Visuals after changes are applied After

@HackforLABot HackforLABot added this to PR Needs review (Automated Column, do not place items here manually) in Project Board Jun 21, 2024
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!


From your project repository, check out a new branch and test the changes.

git checkout -b terrencejihoonjung-update-team-meeting-data-6920 gh-pages
git pull https://github.com/terrencejihoonjung/website.git update-team-meeting-data-6920

@github-actions github-actions bot added role: front end Tasks for front end developers Complexity: Medium P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) P-Feature: Events https://www.hackforla.org/events/ P-Feature: Project Meetings https://www.hackforla.org/project-meetings size: 0.25pt Can be done in 0.5 to 1.5 hours labels Jun 21, 2024
@santisecco
Copy link
Member

Review ETA: 3 PM 6/26/24
Availability: 1-3 PM Wednesday

Copy link
Member

@santisecco santisecco left a comment

Choose a reason for hiding this comment

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

Hi @terrencejihoonjung I have a question. My knowledge in front-end is limited.
When running the website with the changes I see in the events page the addition of:
"*This meeting is not weekly, check project link for more details."
However, I see no event with the * in the end like your function does when "isWeekly":false
Could you help me understand that?

@jphamtv
Copy link
Member

jphamtv commented Jun 27, 2024

Hi @jennisung - please provide your Review ETA and Availability when you have a moment. Thanks!

@terrencejihoonjung
Copy link
Member Author

Hi @terrencejihoonjung I have a question. My knowledge in front-end is limited. When running the website with the changes I see in the events page the addition of: "*This meeting is not weekly, check project link for more details." However, I see no event with the * in the end like your function does when "isWeekly":false Could you help me understand that?

Hi @santisecco,

Sure! The first thing I would do is make sure all my changes exist on your fork. This includes the modified JSON in vrms_data.json, checking that I am conditionally rendering "*", a.k.a "&#42", within the insertEventSchedule function (right-col-content.js), and including the isWeekly property within the display_object function (right-col-content.js).

If there's anything about changes made specifically that you want to ask about, I'm happy to explain further!

@jennisung
Copy link
Member

Review ETA: By 10 PM 6/29/24
Availability: Saturday all day

Copy link
Member

@jennisung jennisung left a comment

Choose a reason for hiding this comment

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

Great job! Everything looks good on my end in Docker, and I see the added text at the end of the Online Project Team Meetings page. I also noticed that you updated the team meeting data to incorporate the asterisk (*) notation for non-weekly meetings per issue #6920.

Copy link
Member

@santisecco santisecco left a comment

Choose a reason for hiding this comment

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

@terrencejihoonjung Thanks for the explanation! Yeah my question was more related to the JSON file.
Your explanation is clear, I had kind of understood what you were trying to do with insertEventSchedule but when I visualized in docker I couldn't see the "*" added. However, today I can see it displayed.

Screenshot 2024-06-29 100711

Displayed events seem to change on a week by week basis so last week, maybe, I saw all events that were "weekly" only, so no * added, or my data was not updated. If you look at your screenshot Thursday is different, with one more *.

Still I can see that the function works now also in my end.
Great job, changes to the code are clear and accurate and you provided a clear and concise explanation of the reasons for the changes. You created the pull request with the correct branch.

Thanks for working on this and explaining!

@santisecco santisecco added the ready for merge team needs a senior review either to do some re writing or to approve it for ready for prioritization label Jun 30, 2024
@t-will-gillis t-will-gillis removed the ready for merge team needs a senior review either to do some re writing or to approve it for ready for prioritization label Jun 30, 2024
@t-will-gillis t-will-gillis merged commit dc11c65 into hackforla:gh-pages Jun 30, 2024
10 checks passed
t-will-gillis added a commit that referenced this pull request Jul 2, 2024
@t-will-gillis
Copy link
Member

NOTE: opened #7086 to revert this PR

t-will-gillis added a commit that referenced this pull request Jul 2, 2024
terrencejihoonjung pushed a commit to terrencejihoonjung/website that referenced this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Medium P-Feature: Events https://www.hackforla.org/events/ P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) P-Feature: Project Meetings https://www.hackforla.org/project-meetings role: front end Tasks for front end developers size: 0.25pt Can be done in 0.5 to 1.5 hours
Projects
Archived in project
Project Board
  
PR Needs review (Automated Column, do...
Development

Successfully merging this pull request may close these issues.

Update Team Meeting data on the Events page to show meeting frequency
5 participants