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.

Sign up for GitHub

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

Jump to bottom

disable all-day when clicking on all-day #226

Merged
merged 1 commit into from
Dec 14, 2016
Merged

disable all-day when clicking on all-day #226

merged 1 commit into from
Dec 14, 2016

Conversation

georgehrke
Copy link
Member

please review @nextcloud/calendar @eppfel

@georgehrke georgehrke added the 3. to review Waiting for reviews label Dec 10, 2016
@mention-bot
Copy link

@georgehrke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @raghunayyar, @tcitworld and @jancborchardt to be potential reviewers.

@georgehrke
Copy link
Member Author

ref #72

tcitworld
tcitworld reviewed Dec 10, 2016
View reviewed changes
css/app/eventdialog.css
@@ -120,6 +120,10 @@
width: 60px;
}

.events .events--time[disabled] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's that ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the next line?

Copy link
Member Author

@georgehrke georgehrke Dec 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://developer.mozilla.org/en-US/docs/Web/CSS/pointer-events

click doesn't work on disabled inputs. With pointer-events:none the click on the input goes straight to the parent div

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks !

@georgehrke georgehrke force-pushed the feature/72-partly/click_to_enable_time_input branch from 66cd841 to 6ffa3a9 Compare December 10, 2016 22:07
eppfel
eppfel requested changes Dec 12, 2016
View reviewed changes
Copy link
Member

@eppfel eppfel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works, but the text cursor is not visible (although I can immediately start typing). It's irritating. might missing focus?

@georgehrke georgehrke force-pushed the feature/72-partly/click_to_enable_time_input branch from 6ffa3a9 to dfca14b Compare December 12, 2016 15:25
@georgehrke georgehrke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Dec 12, 2016
@georgehrke georgehrke force-pushed the feature/72-partly/click_to_enable_time_input branch from dfca14b to 1a73502 Compare December 12, 2016 23:56
@georgehrke
Copy link
Member Author

@eppfel please check again :)

@eppfel
Copy link
Member

eppfel commented Dec 13, 2016 •
edited
Loading

Still no text cursor, but with the popup at least with the mouse no second click is needed...

@eppfel
Copy link
Member

eppfel commented Dec 13, 2016

Oh, but the time input is now in a new line, did this happen in master or is this regression?

@georgehrke
Copy link
Member Author

this also happens in master

@georgehrke
Copy link
Member Author

but this might be related to the span, I'll check

@georgehrke
Copy link
Member Author

fixed that :)

@georgehrke georgehrke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 13, 2016
@georgehrke georgehrke force-pushed the feature/72-partly/click_to_enable_time_input branch from 1a73502 to aa7cf99 Compare December 13, 2016 19:55
@eppfel
Copy link
Member

eppfel commented Dec 14, 2016

I approve once more 👍

@georgehrke georgehrke merged commit a900eda into master Dec 14, 2016
@georgehrke georgehrke deleted the feature/72-partly/click_to_enable_time_input branch December 14, 2016 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcitworld tcitworld tcitworld left review comments

@eppfel eppfel eppfel approved these changes

Assignees
No one assigned
Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@georgehrke @mention-bot @eppfel @tcitworld