AutarkLabs / open-enterprise

A suite of apps that includes allocation, dot voting, issue curation, and other planning tools so organizations can collectively budget and design custom reward & bounty systems.
GNU General Public License v3.0
92 stars 54 forks source link

Allocations: add Budget Detail page #1758

Closed chadoh closed 4 years ago

chadoh commented 4 years ago

Fixes #1575

A lot changed to support this. If you have questions about specific changes, you can check individual commits & commit messages, which are quite detailed.

There's a bit in Card/Budget.js where indentation changed. To hide this noise, view the diff with indentation changes hidden.

budget detail

chadoh commented 4 years ago

@javieralaves @stellarmagnet I've requested reviews from both of you because I'd love if you could check out the attached gif and see if you can spot any style issues before this gets merged.

chadoh commented 4 years ago

Thanks, @javieralaves!

I've updated the GIF in the description to show the two improvements. Do you think it looks good and that it makes sense to work these other items in other PRs?

chadoh commented 4 years ago

@javieralaves I was mistaken about this one:

As #1573 was already implemented, I do need to take care of this now. I've updated it to work like this:

clickable cards

This is a big change from the design, because I removed the context menu. The context menu does not work, when the entire card is clickable. It also provides little value on this Overview page, and less value once #1642 is implemented.

I've freestyled the hover styles here. Let me know what you think of all of this.

stellarmagnet commented 4 years ago

I don't necessarily agree with removing the context menu from the budget card. The court has cards + context menu coming up. We should ask Pierre if they intend to enable this functionality (cards being clickable + context menu).

image

ottodevs commented 4 years ago

The context menu does not work, when the entire card is clickable. It also provides little value on this Overview page, and less value once #1642 is implemented.

This seems not quite accurate, a simple stopPropagation in the click handler for the context menu should be enough to fix that situation, here is some reference in case you ask

As a user I really think that saving one click is a good UX design, in this case removing the context menu from the overview is adding an extra click - at least - so I am not fully satisfied about your decision Chad, anyway the designers should have the last word about this, just adding my technical take on this, and my opinion as potential user, in case that makes sense.

javieralaves commented 4 years ago

This seems not quite accurate, a simple stopPropagation in the click handler for the context menu should be enough to fix that situation, here is some reference in case you ask

@ottodevs Does this mean that it's possible to have both a clickable card and a context menu?

That said – As @chadoh points out, once #1642 is implemented, most day to day interactions will be readily available with one or two clicks; regardless of whether there is a context menu or not. I guess this would no longer hold true if we were to add some form of additional functionality that a context menu could provide room for, so sticking to the pattern won't hurt for now.

What I wanted to originally avoid (and still want to avoid) is having the card title be styled with the link color.