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

Migrate project index page to bootstrap #6648

Merged
merged 1 commit into from
Dec 19, 2018

Conversation

saraycp
Copy link
Contributor

@saraycp saraycp commented Dec 17, 2018

The content of the public projects' table is added by javascript. We keep it like
it was with small changes only: the table itself is written by the view not by JS,
we add "Search" placeholder and remove some unnecessary params.

Co-authored-by: David Kang dkang@suse.com

Before:

public projects open build service 2

After:

public projects open build service

@saraycp saraycp added Frontend Things related to the OBS RoR app Bootstrap 🚀 Bootstrap migration labels Dec 17, 2018
@codecov
Copy link

codecov bot commented Dec 17, 2018

Codecov Report

Merging #6648 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6648      +/-   ##
==========================================
+ Coverage   90.62%   90.63%   +<.01%     
==========================================
  Files         469      467       -2     
  Lines       20434    20433       -1     
==========================================
+ Hits        18519    18520       +1     
+ Misses       1915     1913       -2

@saraycp
Copy link
Contributor Author

saraycp commented Dec 17, 2018

Project index

@coolo
Copy link
Member

coolo commented Dec 18, 2018

Within the production data seems to be a home project (excluding homes looks fine, you get a 2 columned table) that breaks the layout. That problem is also visible on https://build.opensuse.org/project?show_all=true, so just FYI.

screencapture-e120-suse-de-3000-project-2018-12-18-07_16_26

src/api/app/assets/javascripts/webui2/project.js Outdated Show resolved Hide resolved
- @important_projects.each do |project|
%tr
%td
= link_to(project[0], action: :show, project: project[0])
Copy link
Member

Choose a reason for hiding this comment

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

[nitpick] project.first

%td
= link_to(project[0], action: :show, project: project[0])
%td
#{project[1]}
Copy link
Member

Choose a reason for hiding this comment

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

[nitpick] project.second

@saraycp
Copy link
Contributor Author

saraycp commented Dec 18, 2018

Within the production data seems to be a home project (excluding homes looks fine, you get a 2 columned table) that breaks the layout. That problem is also visible on https://build.opensuse.org/project?show_all=true, so just FYI.

@coolo, it is fixed now. When we have long names and titles it breaks the line:

public projects open build service 3

@dmarcoux please read my comments and let me know if you agree.
@Ana06 please have another look

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

Nice job! 💘 although the lint rake task crash. It seems related to the JavaScript code.

@hennevogel
Copy link
Member

The content of the public projects' table is added by javascript. We keep it like it was

Because?

The content of the public projects' table is added by javascript. We keep it like
it was with small changes only: the table itself is written by the view not by JS,
we add "Search" placeholder and remove some unnecessary params.

Co-authored-by: David Kang <dkang@suse.com>
@codecov
Copy link

codecov bot commented Dec 18, 2018

Codecov Report

Merging #6648 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6648      +/-   ##
==========================================
+ Coverage   90.63%   90.63%   +<.01%     
==========================================
  Files         472      467       -5     
  Lines       20441    20433       -8     
==========================================
- Hits        18526    18520       -6     
+ Misses       1915     1913       -2

@coolo
Copy link
Member

coolo commented Dec 18, 2018

The content of the public projects' table is added by javascript. We keep it like it was

Because?

Because server side pagination is out of scope for the bootstrap migration :)

@coolo coolo merged commit be53038 into openSUSE:master Dec 19, 2018
@hennevogel
Copy link
Member

Because server side pagination is out of scope for the bootstrap migration :)

That is not true, if you can avoid migrating this 'pattern' to the new UI. The first "rule" we have for the migration scope is: Identify bad code patterns in the OLD UI and refactor them

@saraycp saraycp deleted the bootstrap_project_index branch March 18, 2019 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bootstrap 🚀 Bootstrap migration Frontend Things related to the OBS RoR app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants