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

Extensionsuitweak #18981

Merged
merged 1 commit into from
Nov 18, 2020
Merged

Extensionsuitweak #18981

merged 1 commit into from
Nov 18, 2020

Conversation

mattwire
Copy link
Contributor

Overview

As pointed out in https://lab.civicrm.org/extensions/stripe/-/issues/275 we are displaying information on the extensions UI in a way that is more helpful to developers than end-users.

Before

Name + extension key shown. Then in details name/key repeated and download location / local path at top.

image

After

Name + description shown. Then in details we don't show name/description again. We move more useful information (links/comments) to the top and less useful information towards the bottom (local path, key etc).
image

Technical Details

template change only.

Comments

I'm sure there are multiple opinions and multiple ways this could be improved further. Hopefully this is a good start :-)

@JoeMurray

@civibot
Copy link

civibot bot commented Nov 16, 2020

(Standard links)

@civibot civibot bot added the master label Nov 16, 2020
CRM/Utils/Money.php Outdated Show resolved Hide resolved
@eileenmcnaughton
Copy link
Contributor

I think the actual change is a small tpl tweak that makes sense - I would note we have had some discussions about moving a community angular extension page extension into being a core extension as it seems like an improvement / leap from what we have and a better thing to build from (non blocking on this but @totten might like to comment)

@mattwire
Copy link
Contributor Author

@seamuslee001 @eileenmcnaughton Sorry! Forgot to rebase out my local patches - now updated.

@eileenmcnaughton
Copy link
Contributor

So I think this is a pretty clear improvement.

The only thing in here that is not just moving information around is the name is removed from the details pane - I assume the reasoning is that it is already in the top of the accordian. I don't have a strong opinion on that - would like to see a couple of +1s or creative emojis just to endorse that part of it

@demeritcowboy
Copy link
Contributor

:shipit:
link

@eileenmcnaughton
Copy link
Contributor

@demeritcowboy was the link to github release policy intended?

@demeritcowboy
Copy link
Contributor

Just to explain what the emoji is about: http://shipitsquirrel.github.com/

@eileenmcnaughton
Copy link
Contributor

Well in the spirit of recent important election results

emoji-icon-glossy-00-00-animals-nature-animal-bird-kakapo-parrot-72dpi-forPersonalUseOnly

https://www.emoji.com/brand-icons/icons/kakapo+parrot-emoji-6625/

https://www.stuff.co.nz/environment/123404888/kkp-crowned-winner-of-bird-of-the-year-2020
the kākāpō, the original “okay boomer” for the male’s deep booming call, bounded into the lead with most votes overall once the preferential voting system was applied.

@seamuslee001
Copy link
Contributor

Based on the feedback on this PR I'm going to add merge ready and if no one objects will merge in a couple of days

@seamuslee001 seamuslee001 added the merge ready PR will be merged after a few days if there are no objections label Nov 18, 2020
@wmortada
Copy link
Contributor

Seems like a useful improvement to me.

@totten
Copy link
Member

totten commented Nov 18, 2020

I think the actual change is a small tpl tweak that makes sense - I would note we have had some discussions about moving a community angular extension page extension into being a core extension as it seems like an improvement / leap from what we have and a better thing to build from (non blocking on this but @totten might like to comment)

I agree - this change looks nice to me, and it's pretty small/safe. No need to block on the extension-management extension.

@seamuslee001
Copy link
Contributor

Given all the supportive comments am going to merge this in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants