add promote build to build menu #191

wavezhang · 2018-02-01T01:41:36Z

Add a "Promote Build" menu to build drawer, when click it will toggle environments list display as follows. The deploy environment list is obtain from a modified droneServer api, see this. Deploy parameters are not supported currently.

mavimo · 2018-02-08T08:35:06Z

src/screens/repo/screens/build/menu.js

 								)}
 							</li>
+							<li>
+								{build.status === "success" ? (


Can we use STATUS_SUCCESS const from src/shared/constants/status.js here?

mavimo · 2018-02-08T08:35:20Z

src/screens/repo/screens/build/menu.js

+									</button>
+								):null }
+							</li>
+							{build.status === "success" && this.state.togglePromote ? (


Can we use STATUS_SUCCESS const from src/shared/constants/status.js here?

mavimo · 2018-02-08T08:36:18Z

src/screens/repo/screens/build/menu.js

+								):null }
+							</li>
+							{build.status === "success" && this.state.togglePromote ? (
+								<ul class="sub">{envs.map(function(env) {


Since this item belong to previous section "Promote" maybe we should move it inside the <li>
tag?

mavimo · 2018-02-08T08:37:29Z

src/screens/repo/screens/build/menu.js

 									</button>
 								)}
 							</li>
+							<li>


This <li> item will be empty when status =! success, maybe we should move in condition?

yes， you are right

mavimo · 2018-02-08T08:38:58Z

src/shared/utils/build.js

+	client
+		.restartBuild(owner, repo, build, { fork: true, event: "deployment", "deploy_to": env })
+		.then(result => {
+			displayMessage(tree, "Successfully promote your build");


Since we should have multiple deployment, maybe we should specify the deployment target in message? something like:

displayMessage(tree, `Successfully promote your build to target ${env}`);

mavimo · 2018-02-08T08:39:07Z

src/shared/utils/build.js

+			displayMessage(tree, "Successfully promote your build");
+		})
+		.catch(() => {
+			displayMessage(tree, "Failed to promote your build");


Same as before

mavimo · 2018-02-08T08:40:56Z

@wavezhang nice work! I added some suggestion, let me know what do you think about it.

wavezhang · 2018-02-23T03:37:57Z

@mavimo I have update my commits following your suggestion

mavimo · 2018-02-23T07:16:30Z

src/screens/repo/screens/build/menu.js

 										<span>Promote Build</span>
 									</button>
 								):null }
+                                {build.status === STATUS_SUCCESS  && this.state.togglePromote ? (


You are mixing TAB & spaces, coding standards used in drone-ui need to use tab, can you run linter (execute yarn lint command from project root) and fix errors? Thx!

mavimo · 2018-02-23T07:18:14Z

src/shared/components/drawer/drawer.less


+	ul {
+		li {
+			margin: 0px;


This should be not needed since we already set it on li element at line 102.

mavimo · 2018-02-23T07:18:29Z

src/shared/components/drawer/drawer.less

+	ul {
+		li {
+			margin: 0px;
+			padding: 0px;


I'm not sure why we are going to remove padding here, having padding to simulate indentation should help to understand "tree" structure

I remove the indentation to algin all icons and text，which i think is more pretty. This is a expandable view, so the "tree" structure is obvious.

without indentation

with indentation

mavimo · 2018-02-23T07:23:08Z

src/screens/repo/screens/build/menu.js

+                                        <input type="text" value={this.state.customEnv}
+                                               onClick={event => event.stopPropagation()}
+                                               onChange={this.updateCustomEnv.bind(this)}
+                                               placeholder="input and then click play icon">


Placeholder description should indicate the content of field and is not intended to be a explanation on how to use it, text should be something like: Deployment target (eg: staging)

mavimo · 2018-02-23T07:26:33Z

@wavezhang some more note (PS: let me know if there is that I can do to help on finalize this PR, I'll like it and your work on it 🥇)

mavimo · 2018-02-25T16:32:43Z

@wavezhang I see also that the notification message do not include the deployment target (comment 1 and comment 2), there is some reason why we should not include it?

bradrydzewski · 2018-02-25T18:43:00Z

I think the general consensus is that we need to be moving things out of the drawer menu (and work toward eliminating the drawer) as opposed to adding more functionality to the drawer. The intention was for the promote button to take the user to a new page, or maybe event some sort of modal.

I just want to set expectations that I'm not currently focused on the UI at the moment, so it may take some time for new UI features to be merged.

ylecuyer · 2018-02-26T15:04:26Z

@bradrydzewski if you don't want to dedicate time to it, there is enough people in the drone team on github to delegate the UI support to

nsiniakevich · 2018-03-19T13:07:02Z

@bradrydzewski is that possible to delegate UI merges to someone else in drone team like @ylecuyer said?

izumskee · 2018-03-28T06:53:37Z

It's very useful feature. Hope it will be added soon.

Wagos · 2018-04-06T07:11:47Z

This is really important feature for us. @bradrydzewski When are you going to add this? Is there a way to help you to add this feature as soon as possible?

smxsm · 2018-04-06T13:40:44Z

@bradrydzewski Promoting builds is also not available via the build API, right? That would be super-awesome, too!

mavimo · 2018-04-15T09:01:19Z

@wavezhang there are some issue on src/screens/repo/screens/build/menu.js, linter indicate:

  126:30  error    Insert `⏎↹↹↹↹↹↹↹↹↹↹`                                                                                                                                                                                                                                                                                          prettier/prettier
  127:11  error    Replace `return·<li><button·onClick={handlePromote.bind(this,·env)}><PlayIcon·/><span>{env}</span></button>` with `↹return·(⏎↹↹↹↹↹↹↹↹↹↹↹↹<li>⏎↹↹↹↹↹↹↹↹↹↹↹↹↹<button·onClick={handlePromote.bind(this,·env)}>⏎↹↹↹↹↹↹↹↹↹↹↹↹↹↹<PlayIcon·/>⏎↹↹↹↹↹↹↹↹↹↹↹↹↹↹<span>{env}</span>⏎↹↹↹↹↹↹↹↹↹↹↹↹↹</button>⏎↹↹↹↹↹↹↹↹↹↹↹↹`  prettier/prettier
  127:18  error    Missing "key" prop for element in iterator                                                                                                                                                                                                                                                                    react/jsx-key
  128:10  error    Insert `↹↹);⏎↹↹↹↹↹↹↹↹↹↹`                                                                                                                                                                                                                                                                                      prettier/prettier
  129:1   error    Replace `↹↹↹↹↹↹↹↹↹<li><button·onClick={handlePromote.bind(this,·this.state.customEnv)}><PlayIcon` with `↹↹↹↹↹↹↹↹↹↹<li>⏎↹↹↹↹↹↹↹↹↹↹↹<button⏎↹↹↹↹↹↹↹↹↹↹↹↹onClick={handlePromote.bind(this,·this.state.customEnv)}⏎↹↹↹↹↹↹↹↹↹↹↹>⏎↹↹↹↹↹↹↹↹↹↹↹↹<PlayIcon·`                                                           prettier/prettier
  130:1   error    Replace `↹↹↹↹↹↹↹↹↹↹<input·type="text"·` with `↹↹↹↹↹↹↹↹↹↹↹↹<input⏎↹↹↹↹↹↹↹↹↹↹↹↹↹type="text"⏎↹↹↹↹↹↹↹↹↹↹↹↹↹`                                                                                                                                                                                                      prettier/prettier
  133:56  error    Replace `·` with `⏎↹↹↹↹↹↹↹↹↹↹↹↹`                                                                                                                                                                                                                                                                              prettier/prettier
  134:1   error    Replace `↹↹↹↹↹↹↹↹↹</button>` with `↹↹↹↹↹↹↹↹↹↹↹</button>⏎↹↹↹↹↹↹↹↹↹↹`                                                                                                                                                                                                                                           prettier/prettier
  135:9   error    Replace `</ul>)·:·null·` with `↹</ul>⏎↹↹↹↹↹↹↹↹)·:·null`                                                                                                                                                                                                                                                       prettier/prettier

I fixed it on mavimo@e0a06cf please report fixes here, thx :)

mavimo · 2018-04-15T09:45:27Z

Hi @wavezhang @ylecuyer @nsiniakevich @izumskee @Wagos @smxsm,
since we have possibility to use drone with custom UI specifying the DRONE_WWW env that contains the folder with the UI we can integrate our UI changes very easily using a container with only UI exposed as volume.

I create a PoC that include:

#191: Add promote build to build menu
#195: Add ability to page build list
#200: Allow to set events when adding new secret

This can be tested using the following config (supposing to use docker-compose if you use kubernetes or some other orchestration tool please you should use a similar approach):

version: '3'

services:
  drone-server:
    image: drone/drone:0.8.5
    environment:
      - DRONE_WWW=/drone-ui
    volumes:
      - drone-ui:/drone-ui

  drone-ui:
    image: mavimo/drone-ui:next
    volumes:
      - drone-ui:/drone-ui

volumes:
  drone-ui:

NB: the PoC I do is only intended useful for testing purpose, identify issue or improvement needed before PRs are included in mainstream, I do not suppose to do maintenance or have a fork that diverge from official UI, so if you chose to use it please report your feedback on issues on order to make it more usable :)

wavezhang · 2018-04-16T01:18:48Z

@mavimo lint errors fixed :)

hnykda · 2018-12-20T16:19:23Z

May I know what's the status of this?

add promote build to build menu

65c2bee

This was referenced Feb 1, 2018

return deploy envs when get build harness/harness#2324

Closed

Return deploy envs when get build harness/harness#2332

Closed

mavimo suggested changes Feb 8, 2018

View reviewed changes

style fix

04eca32

mavimo suggested changes Feb 23, 2018

View reviewed changes

update indentation & update placeholder text

927f626

remove sub ul li style

a73a5d1

This was referenced Apr 15, 2018

Allow to set events when adding new secret #200

Merged

add ability to page build list #195

Merged

fix lint errors

dda342a

bradrydzewski closed this Apr 22, 2019

add promote build to build menu #191

add promote build to build menu #191

Uh oh!

Conversation

wavezhang commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mavimo commented Feb 8, 2018

Uh oh!

wavezhang commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wavezhang Feb 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mavimo commented Feb 23, 2018

Uh oh!

mavimo commented Feb 25, 2018

Uh oh!

bradrydzewski commented Feb 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ylecuyer commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nsiniakevich commented Mar 19, 2018

Uh oh!

izumskee commented Mar 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Wagos commented Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smxsm commented Apr 6, 2018

Uh oh!

mavimo commented Apr 15, 2018

Uh oh!

mavimo commented Apr 15, 2018

Uh oh!

wavezhang commented Apr 16, 2018

Uh oh!

hnykda commented Dec 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

wavezhang commented Feb 1, 2018 •

edited

Loading

wavezhang commented Feb 23, 2018 •

edited

Loading

wavezhang Feb 24, 2018 •

edited

Loading

bradrydzewski commented Feb 25, 2018 •

edited

Loading

ylecuyer commented Feb 26, 2018 •

edited

Loading

izumskee commented Mar 28, 2018 •

edited

Loading

Wagos commented Apr 6, 2018 •

edited

Loading