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

Streamline UI for creating activities #2585

Merged
merged 101 commits into from Sep 27, 2022
Merged

Conversation

tiltec
Copy link
Member

@tiltec tiltec commented Sep 19, 2022

Adds a button to create new activities and series on the group activities page! Behavior is quite similar to the "Manage Activities" page, in fact I reused some of the components.

Each activity also now has a pencil button for editing!

Creating new activities

You click the "plus" button, then select the activity type. A dialog opens, select the place and if a series should be created.

streamline.webm

Editing activities

Click the small "pencil" icon on an activity, then choose from the menu if you want to edit the activity or the whole series (only if the activity is part of a series). There's also the option to jump to the "Manage Activities" page of the store for better overview.

streamline.edit.webm

nicksellen and others added 30 commits August 29, 2022 22:39
So back works properly..
... event if the feature is not enabled
I changed how the backend handles that...
@tiltec
Copy link
Member Author

tiltec commented Sep 26, 2022

This branch has some commits from other branches listed, but it should merge nicely.

I have two test failures that I couldn't resolve today:

  • ActivityCreateButton.spec.js gives "Cannot setup services without a current vue instance :/"
  • GroupActivities.spec.js fails the filter test, but filtering still works when I try it out

@nicksellen
Copy link
Member

Review from clicking around:

edit activity menu

Really nice!

When it's a one-time-activity, it still says "Only this activity" which was a bit confusing to me. I first thought that it's part of a series, but just didn't let me edit the series (hence also linking me to manage activities), as the "only this activity" sort of implied there could be more.

So, maybe if it's a one-time-activity it just says "This activity" instead of "Only this activity" ... perhaps having a little icon to show that something is part of a series is useful too?

also, should disable the edit icon when in preview mode.

activity create modal

the place could use outline style on the place select, as this is what we've moved things to generally.

the place list looks a random order? couldn't tell by looking, and no autocomplete feature (which is probably needed for some groups).

after creating, if it wasn't visible on the page behind me, it felt like nothing had happened, ideally we might scroll to it, but maybe a toast is good enough, to give some indication something happened.

Copy link
Member

@nicksellen nicksellen left a comment

Choose a reason for hiding this comment

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

Really nice! I think a big improvement for managing activities...

Approved! ... and whichever comments/suggestions you want to act on. ⭐

unelevated
padding="0px 13px"
:title="$t('BUTTON.CREATE')"
>
Copy link
Member

Choose a reason for hiding this comment

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

I don't love the non-round-ness of this button, the "place edit" circular button looks nicer to me. I don't think this needs changing though, would make more sense as a general UI-rationalization.

:options="places.map(({ name, id, placeType }) => ({ label: name, value: id, icon: getPlaceTypeById(placeType).icon }))"
:label="$t('CREATEACTIVITY.PLACE')"
emit-value
map-options
Copy link
Member

Choose a reason for hiding this comment

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

add outline as in other comment.

Copy link
Member

Choose a reason for hiding this comment

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

... and maybe ordered
... and maybe autocomplete

Copy link
Member

Choose a reason for hiding this comment

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

... or even better, if we can make a PlaceSelect component - such that it can be used for the activity filter too (that already has autocomplete, ordering, and "show favourite places at the top" feature), all of which would also be useful here.

your one has the added "place type icon" feature which would also be nice for the activity filter.

onSuccess () {
isOpen.value = false
},
})
Copy link
Member

Choose a reason for hiding this comment

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

I think I did use this pattern in places (passing the opts like onSuccess through), I actually came to prefer using the mutateAsync function, then having a local function call it, e.g.

const {
  mutateAsync: createActivity,
  status: createActivityStatus,
  reset: resetNewActivity,
} = useCreateActivityMutation()

async function saveNewActivity (data) {
  await createActivity(data)
  isOpen.value = false
}

... which can then keep the onSuccess usable inside the mutation definition if there is a reason to do something at that level (which there sometimes is).

(Additionally, the non-async mutate function can take options, including onSuccess, but with a caveat, which is in-part what led me to prefer the async method)

Copy link
Member

Choose a reason for hiding this comment

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

(... not suggesting we need to change the style right now though)

src/activities/components/ActivityCreateButton.vue Outdated Show resolved Hide resolved
src/activities/components/ActivityItem.vue Show resolved Hide resolved
@tiltec
Copy link
Member Author

tiltec commented Sep 27, 2022

Thanks, good to know! I made some changes, gonna fix the tests and then merge :)

@codecov
Copy link

codecov bot commented Sep 27, 2022

Codecov Report

Merging #2585 (16ac4cc) into master (68b0cfc) will increase coverage by 0.42%.
The diff coverage is 97.43%.

@@            Coverage Diff             @@
##           master    #2585      +/-   ##
==========================================
+ Coverage   76.43%   76.86%   +0.42%     
==========================================
  Files         360      362       +2     
  Lines       38005    38480     +475     
  Branches     1868     1899      +31     
==========================================
+ Hits        29048    29576     +528     
+ Misses       8957     8904      -53     
Impacted Files Coverage Δ
src/group/components/GroupEdit.vue 0.00% <ø> (ø)
src/maps/components/KMap.vue 97.71% <ø> (+0.55%) ⬆️
src/activities/components/ActivityEditButton.vue 94.30% <94.30%> (ø)
src/activities/components/ActivityCreateButton.vue 98.59% <98.59%> (ø)
src/activities/components/ActivityEdit.vue 83.58% <100.00%> (+0.22%) ⬆️
src/activities/components/ActivityItem.vue 95.77% <100.00%> (+0.02%) ⬆️
src/activities/components/ActivitySeriesEdit.vue 76.00% <100.00%> (+0.40%) ⬆️
src/activities/mutations.js 92.68% <100.00%> (ø)
src/activities/pages/ActivitiesManage.vue 86.64% <100.00%> (+0.04%) ⬆️
src/activities/pages/GroupActivities.vue 97.26% <100.00%> (+0.02%) ⬆️
... and 8 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@tiltec tiltec merged commit efe8f52 into master Sep 27, 2022
@tiltec tiltec deleted the streamline-activity-creation branch September 27, 2022 17:44
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.

None yet

2 participants