mission view redesign #61
Conversation
| } | ||
|
|
||
| uasort($units, fn (Unit $a, Unit $b) => $a->getPosition() <=> $b->getPosition()); | ||
| return $this->unitsWithRSVPs = $units; |
There was a problem hiding this comment.
Not a big fan of the assign-return, just split them up.
| return $this->unitsWithRSVPs = $units; | |
| $this->unitsWithRSVPs = $units; | |
| return $this->unitsWithRSVPs; |
| private function loadRSVPs(): void | ||
| { | ||
| if (!empty($this->rsvpsByUserId)) { | ||
| return; | ||
| } | ||
|
|
||
| foreach ($this->mission->getRsvps() as $rsvp) { | ||
| if ($user = $rsvp->getUser()) { | ||
| $this->rsvpsByUserId[$user->getId()] = $rsvp; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Change this function to also return $this->rsvpsByUserId (maybe also change its name to match it). And then everywhere where this function is used, don't directly access $this->rsvpsByUserId anymore.
That way it's very clear in the usages what it returns, and it's not some magic private variable.
There was a problem hiding this comment.
Perhaps it's a little overkill to have this in a separate twig file? Unless there's a good reason for it, like it's being used in multiple locations?
| {% block tabs %} | ||
| <button type="button" class="btn-outlined w-100 mt-2" data-tab-id="brief" >Briefing</button> | ||
| <button type="button" class="btn-outlined w-100 mt-2" data-tab-id="attendance" >Attendance</button> | ||
| <button type="button" class="btn-outlined w-100 mt-2" data-tab-id="aar" >After Action Reports</button> |
There was a problem hiding this comment.
AARs tab should not be visible if user can not view after action reports.
| </div> | ||
| </div> | ||
| <div id="aar"> | ||
| {% if can('view_after_action_reports', mission.operation) and not mission.afterActionReports.empty %} |
There was a problem hiding this comment.
Move permission check around the entire tab panel
| <div id="attendance"> | ||
| <div id="attendance"> | ||
| {{ component('Forumify\\Perscom\\MissionRoster', {mission: mission}) }} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Not sure why there's 2 divs with the same ID
| <div id="attendance"> | |
| <div id="attendance"> | |
| {{ component('Forumify\\Perscom\\MissionRoster', {mission: mission}) }} | |
| </div> | |
| </div> | |
| <div id="attendance"> | |
| {{ component('Forumify\\Perscom\\MissionRoster', {mission: mission}) }} | |
| </div> |
| <div id="brief"> | ||
| <div> | ||
| <div class="box"> | ||
| <h3>{{ 'perscom.mission.mission'|trans }}</h3> | ||
| <p class="mb-6"> | ||
| {{ 'perscom.mission.start'|trans }} | ||
| <span class="text-bold">{{ mission.start|format_date(true) }}</span> | ||
| </p> | ||
| {% if mission.end %} | ||
| <p class="mb-2"> | ||
| {{ 'perscom.mission.end'|trans }} | ||
| <span class="text-bold">{{ mission.end|format_date(true) }}</span> | ||
| </p> | ||
| {% endif %} | ||
| {{ mission.briefing|rich }} | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Seems to be too much nesting?
| <div id="brief"> | |
| <div> | |
| <div class="box"> | |
| <h3>{{ 'perscom.mission.mission'|trans }}</h3> | |
| <p class="mb-6"> | |
| {{ 'perscom.mission.start'|trans }} | |
| <span class="text-bold">{{ mission.start|format_date(true) }}</span> | |
| </p> | |
| {% if mission.end %} | |
| <p class="mb-2"> | |
| {{ 'perscom.mission.end'|trans }} | |
| <span class="text-bold">{{ mission.end|format_date(true) }}</span> | |
| </p> | |
| {% endif %} | |
| {{ mission.briefing|rich }} | |
| </div> | |
| </div> | |
| </div> | |
| <div id="brief" class="box"> | |
| <h3>{{ 'perscom.mission.mission'|trans }}</h3> | |
| <p class="mb-6"> | |
| {{ 'perscom.mission.start'|trans }} | |
| <span class="text-bold">{{ mission.start|format_date(true) }}</span> | |
| </p> | |
| {% if mission.end %} | |
| <p class="mb-2"> | |
| {{ 'perscom.mission.end'|trans }} | |
| <span class="text-bold">{{ mission.end|format_date(true) }}</span> | |
| </p> | |
| {% endif %} | |
| {{ mission.briefing|rich }} | |
| </div> |
| <div class="flex gap-2" {{ stimulus_controller('forumify/forumify-platform/tabs') }}> | ||
| <div class="w-20"> |
There was a problem hiding this comment.
This does not seem mobile responsive? You should use grid. See templates/frontend/components/course_class/class.html.twig as example.
| <div class="card-body text-center"> | ||
| {% if mission.operation.image %} | ||
| <div class="border-b"> | ||
| <img src="{{ asset(mission.operation.image, 'forumify.asset') }}" width="250px" height="auto"> |
There was a problem hiding this comment.
Add class="rounded", and set width/height through style. See class.html.twig
2 participants
PR contains a redesign of the mission page.