dbeck121 / CPI-Helper-Chrome-Extension

56 stars 19 forks source link

New feature #167

Closed Incpi closed 5 months ago

Incpi commented 5 months ago

Hello,

Let's have a look at changes,


Reason for draft is for your thought. Please help me as i need your critics to enhance it.

Which can be used as given above.

Cloud button on top right corner. Which is accessible across Tool not limited to Just Design. Ignore UI inside of popup.

Thanks, Omkar

dbeck121 commented 5 months ago

@incpi

Thanks for the pull request.

I like the idea of the new Icon next to the user. Thats cool. There could be again the popup data from the chrome plugin icon :-) Also the idea with the debug mode is super nice.

Feature that you can select level (info, warn, debug and trace) is also nice.

Not sure about the "Filter to remove <3ms Steps from Trace as it don't have any data,". I am not sure which impact it has but sounds pretty useful.

So from my side: Would be nice to see your next steps :-)

Maybe sth. that has nothing to do with these changes but sth that made me think:

CleanShot 2024-05-08 at 15 37 23@2x Do you think it might be easy to add another close button on the left side (or move the existing). Often the message sidebar is on top of it so I need to drag it first.

Incpi commented 5 months ago

@DevGregor / @dbeck121 ,

Hey Guyz, Looks 2 different solution are dropped almost at same time. what a coincidence. Thank you. I haven't tested your plugin from comments but seems like similar situation from my own when i directly try to do it with fields.

For Credentials, So This button will be visible at all pages. [You can add details which are needed across tenant in future]. You can get also category wise search in search creds: e.g. User Creds name given is 1,

Downside

Hope this clears your doubt maybe. Looks like essay sorry..... ;) Let me kw your thoughts.

Thanks Omkar.

DevGregor commented 5 months ago

@incpi I have thought about this approach as well with the copy and paste and I do think it is a convenient solution because:

I was checking the issue #151 that was posted, where it was wished to be a dropdown, so I tried to make this possible.

So far the issues I encountered were:

DevGregor commented 5 months ago

@incpi I thought about this again and the only thing I see "against" this is the fact that it is basically what we already have. We can already navigate to the Security Materials or Key Store in a new tab by using these links: image From there SAP has already provided a search help where you can copy the name/aslias from: image

fippu82 commented 5 months ago

@incpi Regarding the Debug mode: I assume you mean the CPI Helper's own debug mode, right? Maybe I misunderstand it, but why would this need to be so prominently accessible via a new shiny icon? I mean, it's only something CPI Helper devs would need, not an end user, isn't it? So I would put it into the CPI Helper Settings and clearly mark it with something like "For CPI Helper Developers".

Incpi commented 5 months ago

Hello @fippu82 ,

Thank you and sorry that is a bit misleading.

Yes I mean it's a for cpi helper own mode. We can mark it for cpi helper.

Settings from popup, then listener only load at initialization of tab, i dont thing we are constantly checking between popup settings and tab synchronise. So thus i added separately.

It's good to have instead of polling for value change.

I will check if already have.

Hope this helps. Omkar

fippu82 commented 5 months ago

Thanks, @incpi for the clarification. I understand your intention and the reason for a new button now. It makes sense to improve the way to control the debug mode, also for users that are reporting issues to us. Still, in my opinion, the button is too prominent and obvious for something that is only used by devs or in case of issues. I have another idea: what about implementing it as a plugin that can be activated in case it's needed? @dbeck121 @DevGregor What do you guys think?

DevGregor commented 5 months ago

Thanks, @incpi for the clarification. I understand your intention and the reason for a new button now. It makes sense to improve the way to control the debug mode, also for users that are reporting issues to us. Still, in my opinion, the button is too prominent and obvious for something that is only used by devs or in case of issues. I have another idea: what about implementing it as a plugin that can be activated in case it's needed? @dbeck121 @DevGregor What do you guys think?

We already have an example plugin for Developers, so I think it could be added there maybe?

Incpi commented 5 months ago

Hello, IMO it's core feature. Hence, as a plugin it's not make sense,

Hope this helps

fippu82 commented 5 months ago

@incpi Yes, I agree that a plugin is not that suitable neither. I pulled your branch and tested it. The new button looks good and less prominent! I also think that we can use the button with its popup in the future to place similar global (non-IFlow related) features.

I have proposed additional description for users, here: [https://github.com/incpi/CPI-Helper/pull/10].

Incpi commented 5 months ago

Popup changed.

Have mix of earlier & new logic (manual + auto timer based closing). Additionally, on hover popup timer will be stopped so you can have your time to read error details.

Also, at a time only one popup will be shown all visible will be closed not like before.

(should I move popup to right - top corner)? what's your opinion, (My opinion is to keep as of right now, lot of people is use to by now to look at bottom right.) Preview: image

@dbeck121 ,now more visually informed popup, Give a try and let me know this helps.

fippu82 commented 5 months ago

Hey Omkar @incpi, I think the new look is fantastic, thank you! I would probably leave it in the bottom right corner, as I'm also used to it. But probably doesn't really matter.

dbeck121 commented 5 months ago

Popup changed.

  • simple logic with kind of template. removed appending children, For success no message shown, Custom props still same as earlier(no changes).

  • Philippe, added separately to give intent to do action. @DevGregor i made 1 small change to your plugin. Interms of color class.

  • For plugins, as of now let's go with this. I don't want to mess with activate/deactivate plugin with headless separate page. If any requirement arise then we will do. #169 -- for last message. as of now, No. If you wish to then go ahead.

  • Global popup i moved to diff, script

Have mix of earlier & new logic (manual + auto timer based closing). Additionally, on hover popup timer will be stopped so you can have your time to read error details.

Also, at a time only one popup will be shown all visible will be closed not like before.

(should I move popup to right - top corner)? what's your opinion, (My opinion is to keep as of right now, lot of people is use to by now to look at bottom right.) Preview: image

@dbeck121 ,now more visually informed popup, Give a try and let me know this helps.

I love it :) great work. Next release will be done kind of celebration with all these new features!

And yes, I agree with the current position.

Incpi commented 5 months ago

Hey, last change from my list in popup

1 more feature to set default log mode

As extra few more bug fixes and what's new Logs, I have one issue if you have better idea , Let me know,

### known Issue: onload button is not 100% loading for initial few seconds.(SAP ui Updates)

Sorry, all things are clubbed in this.

dbeck121 commented 5 months ago

@incpi Great work as always :-) Just add your changes to changelog and tell me when to pull :-)

I am talking to Figaf to release a next version :-)

Incpi commented 5 months ago

@dbeck121 already done, merge whenever u want..😁

dbeck121 commented 5 months ago

Oh no, merge conflicts :)

I am only on my mobile right now. Will check later!

Probably that was me :)

dbeck121 commented 5 months ago

merged!