Skip to content

Add edit project button on project detail page #2823

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

Merged
merged 2 commits into from
Jan 25, 2018
Merged

Conversation

di
Copy link
Member

@di di commented Jan 22, 2018

Fixes #2810.

The button is only shown for logged-in project owners via a client-side include.

@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 23, 2018

thanks for this @di.

One flaw in the template - if a project does not have a release.summary then the whole grey horizontal bar will not show, including the edit button.

In this case, we want the bar to show, but with the edit button alone.

Therefore the logic for the whole horizontal-section should be (excuse my pseudo code) if project.release OR edit project button.

@di
Copy link
Member Author

di commented Jan 23, 2018

@nlhkabu Unfortunately it's not that simple, because the edit project button come from a completely separate request. We only know whether it will be displayed or not within the edit-project-button.html template, not within the parent.

A few things we could do:

  • Always show the grey bar, and show some fallback text if there is no summary instead of hiding it, like "No description provided."
    • Downside: Can't save that little bit of screen real-estate in the edge case where the project has no summary and the button isn't there
  • Move the "Edit Project" button somewhere else
    • Downside: I don't know where else it could go
  • Move the entire summary & grey bar into the edit-project-button.html templage
    • Downside: I'd rather keep the client-side-include as simple as possible

I think the first one makes the most sense to me, thoughts?

@nlhkabu nlhkabu force-pushed the edit-project-button branch from b0a535a to ef0903b Compare January 25, 2018 06:55
@nlhkabu nlhkabu force-pushed the edit-project-button branch from ef0903b to 8fea9d0 Compare January 25, 2018 07:07
@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 25, 2018

Thanks @di - I've updated the template to use fallback text - the first option you suggested. I've also done a quick style update to ensure this looks good / stacks well on mobile, with and without a button :)

Desktop with button

screenshot from 2018-01-25 07-08-01

Desktop without button

screenshot from 2018-01-25 07-07-40

Mobile with button

screenshot from 2018-01-25 07-08-12

Mobile without button

screenshot from 2018-01-25 07-08-20

@nlhkabu nlhkabu requested a review from ewdurbin January 25, 2018 07:10
@nlhkabu
Copy link
Contributor

nlhkabu commented Jan 25, 2018

@ewdurbin I believe this is ready. Can you please review and merge if you are satisfied? Thx

@ewdurbin ewdurbin merged commit 5498298 into master Jan 25, 2018
@ewdurbin ewdurbin deleted the edit-project-button branch January 25, 2018 11:31
@ewdurbin
Copy link
Member

Looks great, thanks @nlhkabu!

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.

3 participants