Skip to content
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

Confirm modal: Exclamation mark out of place #15778

Open
tobiasKaminsky opened this issue May 28, 2019 · 9 comments
Open

Confirm modal: Exclamation mark out of place #15778

tobiasKaminsky opened this issue May 28, 2019 · 9 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 26-feedback design Design, UI, UX, etc. papercut Annoying recurring issue with possibly simple fix. technical debt

Comments

@tobiasKaminsky
Copy link
Member

From: #15774 (comment)

Looks a bit blurry to me, and ! is out of place?

@ChristophWurst

yes, but that is an issue with the confirm modal in general. mind opening a ticket for that and pinging our designers? thx

@nextcloud/designers

@tobiasKaminsky tobiasKaminsky added bug design Design, UI, UX, etc. labels May 28, 2019
@skjnldsv
Copy link
Member

Looks a bit blurry to me, and ! is out of place?

Your window screen have an impair height. So the translate is antialiasing on the subpixel :)
We should indeed try to center vertically the popup differently :)

@jancborchardt
Copy link
Member

The icon itself is out of place here, it does not need to be there.

Additionally the buttons should not say "Yes" and "No" but be more descriptive like "Wipe data from this device" and "Keep data".

@MorrisJobke MorrisJobke added the good first issue Small tasks with clear documentation about how and in which place you need to fix things in. label May 31, 2019
@MorrisJobke MorrisJobke added this to the Nextcloud 17 milestone May 31, 2019
@skjnldsv skjnldsv added the 0. Needs triage Pending check for reproducibility or if it fits our roadmap label Jun 12, 2019
@georgehrke
Copy link
Member

This is actually not that trivial. Definitely no good first issue.

OC.Dialogs.confirm, etc. only allows you either an OK button or YES/NO. It doesn't allow custom labels.
cc @ChristophWurst Something we should do when porting it to vue.

@georgehrke georgehrke removed the good first issue Small tasks with clear documentation about how and in which place you need to fix things in. label Jun 27, 2019
@GretaD GretaD removed their assignment Jun 27, 2019
@pierlon
Copy link
Member

pierlon commented Jun 27, 2019

OC.Dialogs.confirm is a wrapper for OC.Dialogs.message, so calling OC.Dialogs.message with the custom buttons could be a workaround for now.

@georgehrke
Copy link
Member

georgehrke commented Jun 27, 2019

@pierlon Please check the code :)
You can only set an OK button or YES/NO. Even when using OC.Dialogs.message directly

@pierlon
Copy link
Member

pierlon commented Jun 28, 2019

Yep you're right, overlooked that. Perhaps you could try changing the text after its created:

OC.dialogs.confirm('Text', 'Title', null, true).done(() => {
    const buttons = $('.oc-dialog-buttonrow button')

    buttons.eq(0).text('Keep data')
    buttons.eq(1).text('Wipe data')
})

@georgehrke
Copy link
Member

Possible yes, but quite hacky and definitely nothing we should teach new contributors 😉
There are plans to have a clean, no-jquery implementation in our vue-components. We should just make sure to allow custom button labels there and migrate all usages of OC.Dialogs to our vue component.

@rullzer rullzer modified the milestones: Nextcloud 17.0.4, Nextcloud 17.0.5 Mar 11, 2020
@rullzer rullzer modified the milestones: Nextcloud 17.0.5, Nextcloud 17.0.6 Mar 23, 2020
@rullzer rullzer removed this from the Nextcloud 17.0.7 milestone Jun 4, 2020
@rullzer rullzer added this to the Nextcloud 17.0.8 milestone Jun 4, 2020
@MorrisJobke MorrisJobke added the papercut Annoying recurring issue with possibly simple fix. label Aug 20, 2020
@skjnldsv skjnldsv added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Sep 9, 2020
@skjnldsv skjnldsv removed this from the Nextcloud 20 milestone Sep 9, 2020
@szaimen
Copy link
Contributor

szaimen commented Jan 9, 2023

Hi, please update to 24.0.8 or better 25.0.2 and report back if it fixes the issue. Thank you!

@szaimen szaimen added needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 1. to develop Accepted and waiting to be taken care of labels Jan 9, 2023
@tobiasKaminsky
Copy link
Member Author

image

With latest master (~NC26)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 26-feedback design Design, UI, UX, etc. papercut Annoying recurring issue with possibly simple fix. technical debt
Projects
None yet
Development

No branches or pull requests

9 participants