[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #1633: feat: add the service page upstream select option

2021-04-26 Thread GitBox 


liuxiran commented on a change in pull request #1633:
URL: https://github.com/apache/apisix-dashboard/pull/1633#discussion_r619735930



##
File path: web/src/components/Upstream/components/UpstreamSelector.tsx
##
@@ -59,6 +59,9 @@ const UpstreamSelector: React.FC = ({ onChange, list = 
[], disabled, requ
 {item.name}
   
 ))}
+{
+  formatMessage({ id: 'page.upstream.step.select.upstream.select.none' 
})
+}

Review comment:
   we already have `None` select.option in line 50
   ```suggestion
   
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #1633: feat: add the service page upstream select option

2021-03-25 Thread GitBox 


liuxiran commented on a change in pull request #1633:
URL: https://github.com/apache/apisix-dashboard/pull/1633#discussion_r601934240



##
File path: web/src/components/Upstream/UpstreamForm.tsx
##
@@ -667,6 +667,9 @@ const UpstreamForm: React.FC = forwardRef(
   name: formatMessage({ id: 
'page.upstream.step.select.upstream.select.option' }),
   id: '',
 },
+{

Review comment:
   Hi @stu01509 
   1. turn to this line 
https://github.com/apache/apisix-dashboard/pull/1633/files#diff-0029901a283985aea1e8e47cfb2d45c9c8eef8cb392e12ed31720390a22ec269R664,
 you will see 
   
![image](https://user-images.githubusercontent.com/2561857/112560691-b0184900-8e0e-11eb-8e7e-8991ceb3761f.png)
   
   this is the `None` option item, and you will find that it is controlled by 
`required` parameter
   3. for the `upstreamForm` component, `required` is a props item, see
   
![image](https://user-images.githubusercontent.com/2561857/112560919-1f8e3880-8e0f-11eb-923b-a23aa5b72d60.png)
   
   4. our create service page used `upstreamForm` component, see
   
![image](https://user-images.githubusercontent.com/2561857/112561354-ff12ae00-8e0f-11eb-9295-595ddc8ce3d2.png)
   
   you can try to change the `required` param to `required={false}`, that will 
solve your problem.
   
   just try it ^_^, looking forward to your reply, thanks




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #1633: feat: add the service page upstream select option

2021-03-23 Thread GitBox 


liuxiran commented on a change in pull request #1633:
URL: https://github.com/apache/apisix-dashboard/pull/1633#discussion_r599481208



##
File path: web/src/components/Upstream/UpstreamForm.tsx
##
@@ -667,6 +667,9 @@ const UpstreamForm: React.FC = forwardRef(
   name: formatMessage({ id: 
'page.upstream.step.select.upstream.select.option' }),
   id: '',
 },
+{

Review comment:
   Hi @stu01509 
   I noticed that we already have `None` option, see
   
https://github.com/apache/apisix-dashboard/pull/1633/files#diff-0029901a283985aea1e8e47cfb2d45c9c8eef8cb392e12ed31720390a22ec269R664
   
   and this will be controlled by `required` param, you can try to pass 
`required={false}` in 
   
https://github.com/apache/apisix-dashboard/blob/9c27b57dc5d2549706a8348f1a036743246f0337/web/src/pages/Service/components/Step1.tsx#L57
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org