This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 229687 - Poor customizer for CSS preprocessors
Summary: Poor customizer for CSS preprocessors
Status: RESOLVED FIXED
Alias: None
Product: web
Classification: Unclassified
Component: HTML Project (show other bugs)
Version: 7.4
Hardware: All All
: P2 normal (vote)
Assignee: Tomas Mysik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-14 03:32 UTC by David Konecny
Modified: 2013-05-30 01:38 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
screenshot (29.81 KB, image/png)
2013-05-15 00:06 UTC, David Konecny
Details
illustrative screenhost (22.58 KB, image/png)
2013-05-15 01:36 UTC, David Konecny
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Konecny 2013-05-14 03:32:16 UTC
I know this is subjective but UI of CSS Preprocessors panel now has "Options..." link at the top right corner and in my honest opinion it looks super ugly: it pushes everything down; the name is cryptic; why is it link and not button?;
Comment 1 Tomas Mysik 2013-05-14 06:49:22 UTC
(In reply to comment #0)
> I know this is subjective but UI of CSS Preprocessors panel now has
> "Options..." link at the top right corner and in my honest opinion it looks
> super ugly:

So, where it should be?

> the name is cryptic

Not sure why you think so (we have Tools > Options) but feel free to suggest a better name, of course.

> why is it link and not button?

Will change it, no problem; just curious - why a button? Sorry if I am missing something, in PHP support we use links everywhere without any problems...

Thanks.
Comment 2 David Konecny 2013-05-15 00:06:58 UTC
Created attachment 134447 [details]
screenshot
Comment 3 David Konecny 2013-05-15 00:49:09 UTC
I attached the screenhost of what it looks like currently. If everybody else think it looks good then I will shut up. :-)

Note also that this screenhost shows a project which:
* has SASS files; and
* does not have SASS compiler set in global options; and
* project node is red with a project problem telling me "Invalid SASS compiler" (why "invalid"? it could just say what the problem is, that is "No SASS compiler found").
yet Project Properties panel does not tell user that compiler is missing. I would expect to see an error message here too.

Another observations:
* in order to specify "compile SASS files into the same folder" user has to figure out to enter "/:/" or something similar - that's not intuitive at all
* in offline email MarekF mentioned that it took him a while to grasp CSS output folder semantic. Blame me as I suggested to keep it as single field - it is similar to what user has to specify via command line sass parameter. But perhaps it is time to change that too.
Comment 4 David Konecny 2013-05-15 01:36:50 UTC
Created attachment 134448 [details]
illustrative screenhost

Just an idea how content of the SASS panel could look like:

* 'Configure' button opens Options dialog; error that compiler is not set is shown only if user ticks "Compile Sass on Save"

* when "use different output" is not checked the table with mappings is empty and disabled
Comment 5 Tomas Mysik 2013-05-15 05:46:09 UTC
Changing summary since now it is definitely not about any link anymore.

I am definitely fine with improving the panel but first we need to agree on its final form - Liza, could you please help us with it?

@David: Does that mean that each project can have its own Sass/LESS compiler? Since that is my feeling now from your proposal... Please comment and reopen this issue (and perhaps assign to Liza?).

Thanks.
Comment 6 Petr Jiricka 2013-05-15 08:12:54 UTC
> Does that mean that each project can have its own Sass/LESS compiler?

That's not my impression. It looks to me like the field is read-only, just mirroring the value in global options. And the only way to change it is through the Configure button.

I like the "use different output" checkbox - makes for a much better default behavior. As for the mapping table, one question: is it as powerful/expressive as the current solution? Or are there things that you can express currently that the table does not allow? Also, do we need Move Up/Move Down buttons for this table?

Cc'ing also Marek as he stirred up this discussion.
Comment 7 Tomas Mysik 2013-05-15 08:27:12 UTC
(In reply to comment #6)
> That's not my impression. It looks to me like the field is read-only, just
> mirroring the value in global options. And the only way to change it is through
> the Configure button.

I see now, yes you are likely right, David will confirm that.

> I like the "use different output" checkbox - makes for a much better default
> behavior. As for the mapping table, one question: is it as powerful/expressive
> as the current solution? Or are there things that you can express currently
> that the table does not allow? Also, do we need Move Up/Move Down buttons for
> this table?

The checkbox has a "problem" - right now, we show project problem if no mapping is set yet; this checkbox will cause that no project problem will be shown (or how to do that? If one opens customizer now, he will see an error that the field is empty).
And yes, we will need Move Up/Move Down buttons as well since we use "the first rule that matches wins" (I have already written it on the mailing list when we have discussed it).
Comment 8 Marek Fukala 2013-05-15 09:07:11 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > That's not my impression. It looks to me like the field is read-only, just
> > mirroring the value in global options. And the only way to change it is through
> > the Configure button.
> 
> I see now, yes you are likely right, David will confirm that.
> 
> > I like the "use different output" checkbox - makes for a much better default
> > behavior. As for the mapping table, one question: is it as powerful/expressive
> > as the current solution? Or are there things that you can express currently
> > that the table does not allow? Also, do we need Move Up/Move Down buttons for
> > this table?
> 
> The checkbox has a "problem" - right now, we show project problem if no mapping
> is set yet; this checkbox will cause that no project problem will be shown (or
> how to do that? If one opens customizer now, he will see an error that the
> field is empty).
> And yes, we will need Move Up/Move Down buttons as well since we use "the first
> rule that matches wins" (I have already written it on the mailing list when we
> have discussed it).
IMHO If there's some reasonable default value (e.g. scss from any path will go to a css file in the same path) then I think the the project problem can go away. 

BTW the new UI proposed by David is a huge improvement, seems very clear to me. Thanks.
Comment 9 Tomas Mysik 2013-05-15 12:42:56 UTC
(In reply to comment #7)
> The checkbox has a "problem" - right now, we show project problem if no mapping
> is set yet; this checkbox will cause that no project problem will be shown (or
> how to do that? If one opens customizer now, he will see an error that the
> field is empty).

Perhaps we could add 2 radio buttons and the default state would be "none selected" - in this case, we would show a project problem.
Comment 10 David Konecny 2013-05-16 04:13:41 UTC
(In reply to comment #6)
> It looks to me like the field is read-only, just
> mirroring the value in global options.

Yes.

> As for the mapping table, one question: is it as powerful/expressive
> as the current solution?

Yes.

(In reply to comment #7)
> The checkbox has a "problem" - right now, we show project problem if no mapping
> is set yet

I was thinking about using something like a three state checkbox - true/false/unset. But we do not really use that UI anywhere else in IDE I think. I wonder whether we should just make it simple for now and simply leave the checkbox by default unchecked and compile SASS into the same folder. And let user change it if they want. That would also mean no project problem for user to deal with.

> And yes, we will need Move Up/Move Down buttons as well since we use "the first
> rule that matches wins" (I have already written it on the mailing list when we
> have discussed it).

We did discuss it and I explained that it does not make sense in practise as most of the projects will need only single folder to folder mapping.

(In reply to comment #8)
> IMHO If there's some reasonable default value (e.g. scss from any path will go
> to a css file in the same path) then I think the the project problem can go
> away. 

Ideally SASS/LESS new file wizard would ask user where to generate CSS and setup the Compiler output folder mapping automatically. In case of an existing project or sudden appearance of SASS/LESS we could try to guess it - I was advocating that for a while but I let it go; should be fairly easy for most of the cases.
Comment 11 Petr Jiricka 2013-05-16 07:39:12 UTC
> I was thinking about using something like a three state checkbox -
> true/false/unset.

I think we should have three states and operate with the "unset" value which would show the project problem - killing this project problem would be dangerous: imagine a situation when the user imports an existing project which uses some mapping to compile SASS to a different folder (via command line tools). If we just used false as the default value, then the IDE would compile SASS files to the same folder, which would result in generated CSS files in 2 places, unwanted files under version control etc. So I think the "unset" value is important.

UI-wise, maybe we don't need a three state checkbox. What about just having regular 2-state checkbox (which would be unchecked for both "false" and "unset" values), and present the project problem as "Please review the SASS compiler output mapping", which would just open the customizer. When the user opens the customizer (either via this project problem, or via regular project properties action) and confirms it with OK, then the initial "unset" value would be changed to false or true. Additionally, we would display an inline warning in this customizer panel saying "Please review the output file mapping and press OK to confirm."
Comment 12 lxlyons 2013-05-16 12:30:26 UTC
Guys:

A tri-state checkbox is not a good fit here.  Can I suggest that we take this out of the bug for the design?  I need a concise summary of the requirements -- what this UI needs to support -- independent of any design proposals, and I can quickly suggest something.

Thanks!
Liza
Comment 13 Petr Jiricka 2013-05-16 19:10:56 UTC
Hi Liza, let me try to summarize:
- There needs to be a way to enter mappings of output directories in the form of "this input directory containing SASS files maps to this output directory of CSS files"
- It should be very convenient to specify the case "just put the output in the same directory as the input file"
- When importing an existing project which may be using some mappings of input to output directories, the user should be guided to specify/review the mapping. While we may be able to guess what the mapping is based on the source structure, we can never be sure that the IDE guessed it correctly, and the user should always review the mapping and correct it if it's wrong. (Currently we use the "project-level warning" for this).

Thanks in advance!
Comment 14 David Konecny 2013-05-16 22:10:16 UTC
(In reply to comment #11)
> What about just having regular 2-state checkbox (which would be unchecked 
> for both "false" and "unset" values)

I had thought about it but it is tricky. What if user does not have any SASS files in their project yet and they open project properties and configure SASS compiler. That would remove "unset" value. Which sounds OK, but how do you distinguish this from a state when user did not configure SASS compiler but had a look at the panel in project customizer and pressed OK - in the second case user did not configure SASS compiler yet we would remove "unset" value too.

(In reply to comment #13)
> Hi Liza, let me try to summarize:

One important aspect I would like to add is that our SASS/LESS compiler is by default enabled. That's what complicates the situation. If it was disabled by default and user had to explicitly enable it first then as part of that step user would have to configure the compiler and workflow would be simple. But that would also assume that user is aware that NetBeans has SASS/LESS compiler support and that users know where to go to enable it and configure it. So instead of that we've decided to keep the compiler always enabled so that once user creates their first SASS file the compiler just works and user gets the generated CSS. Which I still think is good approach. But as Petr described in comment #11 the problem might happen if user has an existing project sources where SASS files are compiled into a different CSS output. In such case we simply do not know where the compile SASS files. Or even if IDE guessed where to compile SASS files we should still let user review such compiler configuration before we execute the compiler for the first time. oJET sources are good example of this case.

In completely new project the New SASS File wizard should ask for CSS output and setup compiler automatically.
Comment 15 Tomas Mysik 2013-05-29 10:31:20 UTC
UI of the Project Properties panel is now as Liza suggested.

http://hg.netbeans.org/web-main/rev/a37cef6cf613

As we agreed with PetrJ, the Project Properties panel now validates also compiler and informs user if compiler is invalid (with a hint to use Configure Executables button). It means, that now, the whole validation of CSS preprocessors (compilers + mappings) is done in only one single panel. Please, have a look at it and feel free to provide your comments.

http://hg.netbeans.org/web-main/rev/e9e54ae0a705

Also please notice that due to issue #228645, sometimes incorrect Sass/LESS file
count is returned and so no validation is done - see issue #228722, comment #5 (or should we do validation no matter whether there are any Sass/LESS files in the project?). I have added logging for it, start NB with [1] to check the number of returned Sass/LESS files for the given project.

Last note: IMHO we should add some info message about the expected format for the Input and Output paths - right now, for new projects, just empty table is shown and I don't think that people will be able to guess what they should put there.

Thanks.
[1] -J-Dorg.netbeans.modules.css.prep.util.level=FINE
Comment 16 Tomas Mysik 2013-05-29 11:17:12 UTC
Please, see issue #230445 for additional information.

Thanks.
Comment 17 Quality Engineering 2013-05-30 01:38:19 UTC
Integrated into 'main-golden', will be available in build *201305292301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/a37cef6cf613
User: Tomas Mysik <tmysik@netbeans.org>
Log: #229687 - Poor customizer for CSS preprocessors