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

Highlight the changeset editor when the comment is empty #4624

Merged

Conversation

ogadaki
Copy link
Contributor

@ogadaki ogadaki commented Dec 20, 2017

Change the style of the changeset editor when the comment is empty to explicitly indicate the user what he has to do.

This PR is a minimal change to fix #4613. It only modifies the CSS for the changeset editor.

I've just seen there is another PR for this issue (#4621) which implements some other features that might be helpfull. I still submit mine in case a simpler fix is preferable (also, it's my first one for iD and I chose a "good first issue").

(closes #4613)

@bhousel
Copy link
Member

bhousel commented Jan 2, 2018

This is perfect, thanks @nnodot 👍

@bhousel bhousel merged commit 11d0ec8 into openstreetmap:master Jan 2, 2018
@althio
Copy link
Contributor

althio commented Jan 3, 2018

Thanks @nnodot for the PR and @bhousel for merging this in.
I like the results with a simple CSS fix.

One question though:
What is the expected behaviour when the changeset comment field is pre-filled by an external application?

@ogadaki
Copy link
Contributor Author

ogadaki commented Jan 3, 2018

@althio I was asking myself the same question. I guess one might want to have the same behavior as the empty comment one's (as in this PR)?

@bhousel
Copy link
Member

bhousel commented Jan 3, 2018

I did not test the behavior with a pre-filled comment, but I'm assuming it would work the same way (that's good!). If the changeset comment field has some value, the field will be classed as present during the update selection.

iD/modules/ui/field.js

Lines 142 to 144 in baeff8f

container
.classed('modified', isModified())
.classed('present', isPresent())

tyrasd added a commit that referenced this pull request Feb 25, 2025
restores behaviour from #4624/#4624, but switch to a neutral emphasis instead of the red color (which might be misinterpreted that the user already made a mistake)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

focus to changeset comment not obvious (enough)
3 participants