-
Notifications
You must be signed in to change notification settings - Fork 806
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
User status bug fixing... #3069
Conversation
camilasan
commented
Apr 6, 2021
- address Kevin's last comments in the PR Status feature #2505
- if there is no emoji in the status message (and for the default "Online, offline, etc" ones), the text is a bit off to the right, not left-aligned with name and server link
- the menu icon for the other apps is missing on the right next to the Talk icon, and Talk icon moved over? Unrelated to status, but visible in the screenshots and also in the release it seems
79ba65e
to
99b3e61
Compare
@er-vin Alrighty then. Never thought about that, but, it should also be
simple to check that the enum index is within bounds and has a
corresponding `QString` assigned to it.
Well, that's why I mentioned "in the middle". Typically sometimes you'd have
something which is still within bounds but without the message you expect. Of
course in other cases you'd have something outside of bounds but this is
easier to catch.
Now, if we are talking about `enum class`, why not make that `enum Status`
an `enum class Status` instead, @camilasan ? :)
Yes good point. Thought I mentioned it in the past but probably forgot.
|
|
@er-vin In one of my past jobs, we were using a command-line tool that would
generate all that's needed for enum (array with strings and conversion
functions). So, adding an enum index in the middle would just be a simple
insertion into an enum template file which is then fed to a command-line
tool during the build :)
Yes, tooling is a way to escape this. We got that for free in some way with
Q_ENUM. The difficulty here is the translated strings, Q_ENUM won't help you
for this and so you're back to square one...
|
223a36b
to
63fbc0f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine. Let's fix QML warnings in 3.2.1.
Looks fine. Let's fix QML warnings in 3.2.1.
This + move stringToEnum and enumToString in an anonymous namespace in the cpp
file. From the other comments I think that one fell through the cracks.
|
/rebase |
Signed-off-by: Camila <hello@camila.codes>
Signed-off-by: Camila <hello@camila.codes>
- Make preDefinedStatus a local var. - Add function to get the status user string. Signed-off-by: Camila <hello@camila.codes>
Cleans the space in the beginning of the string when there is no emoji set. Signed-off-by: Camila <hello@camila.codes>
Signed-off-by: Camila <hello@camila.codes>
Signed-off-by: Camila <hello@camila.codes>
Signed-off-by: Camila <hello@camila.codes>
Signed-off-by: Camila <hello@camila.codes>
Signed-off-by: Camila <hello@camila.codes>
Signed-off-by: Camila <hello@camila.codes>
67a638d
to
1ca0ea4
Compare
AppImage file: Nextcloud-PR-3069-1ca0ea49dd9fee42af0b0a4d8281941afb1f163e-x86_64.AppImage |