gtt-project / redmine_text_blocks

Plugin that adds configurable text blocks for replying to issues
GNU General Public License v3.0
3 stars 3 forks source link

Moved issue's comment 'Select text block...' position #14

Closed sanak closed 3 years ago

sanak commented 3 years ago

Signed-off-by: Ko Nagase nagase@georepublic.co.jp

Supports #13.

Changes proposed in this pull request:

@gtt-project/maintainer

sanak commented 3 years ago

@mopinfish, @dkastl
Well, I will try to find dynamic responsive solution (move template position only in narrow browser width (or mobile OS) case) to avoid to confuse existence users, so please wait for a while.

dkastl commented 3 years ago

I wouldn't spend so much time on this. I think it's better for all screens to show the text template dropdown first.

So just announce it as an improvement, that's enought! There is no need to excuse improvements ;-)

sanak commented 3 years ago

I wouldn't spend so much time on this. I think it's better for all screens to show the text template dropdown first.

So just announce it as an improvement, that's enought! There is no need to excuse improvements ;-)

@dkastl Oh, yes. I will not spend too much time about this, but actually, I felt that too much emphasizing the plugin's control is not good, so I will try responsive way a bit. 😅

sanak commented 3 years ago

Well, I will try MediaQuery + display: inline-flex with order from referring the following Japanese articles.

sanak commented 3 years ago

@mopinfish (CC: @dkastl)
I reverted original commit and added responsive (media query) with display: inline-flex with order, so could you check the diff again ?
Thanks,

mopinfish commented 3 years ago

@sanak changes looks good. But, I am worried about failed test for v4.1.

Don't you have to worry about this?

sanak commented 3 years ago

@mopinfish Oh, I missed it. Thanks for mentioning about it. The error says LoadError: cannot load such file -- /var/test/test_helper, so I guess that some setup is missing, but I will check it a bit, later.

sanak commented 3 years ago

@mopinfish (CC: @dkastl)
Hmm... About the test error, I confirmed that same query was no problem on my local macOS environment, so there may be current using GitHub actions itself issue...

% RAILS_ENV=test bundle exec rake --trace redmine:plugins:test NAME=redmine_text_blocks
Unknown database adapter `postgis` found in config/database.yml, use Gemfile.local to load your own database gems
** Invoke redmine:plugins:test (first_time)
** Execute redmine:plugins:test
** Invoke redmine:plugins:test:units (first_time)
** Invoke db:test:prepare (first_time)
** Invoke db:load_config (first_time)
** Invoke environment (first_time)
** Execute environment
** Execute db:load_config
** Execute db:test:prepare
** Invoke db:test:load (first_time)
** Invoke db:test:purge (first_time)
** Invoke db:load_config 
** Invoke db:check_protected_environments (first_time)
** Invoke db:load_config 
** Execute db:check_protected_environments
** Execute db:test:purge
** Execute db:test:load
** Invoke db:test:load_schema (first_time)
** Invoke db:test:purge 
** Execute db:test:load_schema
** Execute redmine:plugins:test:units
Mocha deprecation warning at /Users/sanak/Projects/gtt/git/redmine/vendor/bundle/ruby/2.6.0/gems/polyglot-0.3.5/lib/polyglot.rb:65:in `require': Require 'mocha/test_unit', 'mocha/minitest' or 'mocha/api' instead of 'mocha/setup'.
** Invoke redmine:plugins:test:functionals (first_time)
** Invoke db:test:prepare 
** Execute redmine:plugins:test:functionals
** Invoke redmine:plugins:test:integration (first_time)
** Invoke db:test:prepare 
** Execute redmine:plugins:test:integration
Run options: --seed 38448

# Running:

........

Finished in 1.242922s, 6.4364 runs/s, 49.8825 assertions/s.
8 runs, 62 assertions, 0 failures, 0 errors, 0 skips

I will try dummy PR for check which is the cause of this issue.

sanak commented 3 years ago

@mopinfish (CC: @dkastl)
The following dummy PR (only change README.md) CI tests failed, so I think that CI tests failure is another problem. https://github.com/gtt-project/redmine_text_blocks/pull/15

Are you okay to merge this once, then create another issue for CI test failure ?

mopinfish commented 3 years ago

@sanak I got about CI Problem. So , I think this issue is LGTM.

sanak commented 3 years ago

@mopinfish Okay, thanks for confirmation. Then I will merge this with squash mode.