joejoe2 / chat-frontend

1 stars 0 forks source link

Private and group chat blockage UI component #1

Closed sergejsvisockis closed 11 months ago

sergejsvisockis commented 1 year ago

As soon as both the private and group chat blockage functionality is implemented on the back-end side (https://github.com/joejoe2/spring-chat/issues/5) it's required to introduce a corresponding Vue component into the UI.

joejoe2 commented 1 year ago

Added setting for private chat at 46517a903df0e5bd19b99c930e53c66818adcff7

sergejsvisockis commented 1 year ago

ok, looks good

sergejsvisockis commented 11 months ago

@joejoe2 Now it remains to add a proper group chat participant blockage capability on the UI. Just like for the private settings was done already.

joejoe2 commented 11 months ago

Since https://github.com/joejoe2/spring-chat/issues/5 was completed, we have to add corresponding UI components for group chat.

sergejsvisockis commented 11 months ago

Fantastic! Sounds good ;)

joejoe2 commented 11 months ago

See cb25061745c8481506994800391b05e42df91333. I also want to change ui in private chat to follow this style.

sergejsvisockis commented 11 months ago

I've left several comments under that revision.

The default boolean value is "false".

joejoe2 commented 11 months ago

It seems there are some redundant reset because showAlert will be closed by user.

I think if the error doesn't contain message, console.log() may be more reasonable.

sergejsvisockis commented 11 months ago

sure, let's leave it like it is.

sergejsvisockis commented 11 months ago

But, I have thought around - I still think we can move this else statement. Boolean default value is false and the text value is blank - ""

joejoe2 commented 11 months ago

I removed that and replaced with console.log 5cd4f48

sergejsvisockis commented 11 months ago

Cool!

sergejsvisockis commented 11 months ago

But IMO it's not a good idea to output an error message into the console.... Instead, these have to be showcased on the UI as a popup or any other way.

It was fine like it was before although we can drop this "else" statement.

joejoe2 commented 11 months ago

You're right. I forgot users can see console if they are smart enough.

sergejsvisockis commented 11 months ago

That's when two backend engineers are tackling the UI :D

joejoe2 commented 11 months ago

Fixed in e77bf0b. Actually this is my first vue/frontend project, so there is a lot of immature code I think ... 😅

sergejsvisockis commented 11 months ago

Totally understand :D I myself have been working with the UI seriously/commercially as a freelancer back in 2012 to 2014. and when I stopped the first Angular version had been released :D Even now remember Angular 0.1.0 xD and that Google I/O presentation.

So, lotta things have changed. For now it is rarely when I am tackling the UI although engineering fundamentals help here since these apply to the UI as well :) And docs of course:) and outdated experience also :D

sergejsvisockis commented 11 months ago

Looks good now :)

sergejsvisockis commented 11 months ago

I think it looks good and for now, I have nothing more to add here at this point. At least in the scope of this feature.

I'm closing the issue. Feel free to reopen if I am missing something.

Thanks a lot for your collaboration! It was fun! :)

sergejsvisockis commented 11 months ago

@joejoe2 I've just noticed that the branch is not merged.

I think we can merge it.

joejoe2 commented 11 months ago

In fact this repo is not for chat app thoroughly, I will rename the repo and split out the content in main branch. Then merge the current branch.

joejoe2 commented 11 months ago

Done. I am closing this issue for now.

sergejsvisockis commented 11 months ago

Sounds good! Thanks! ;)