Page MenuHomeSoftware Heritage
Differential D1732

Add option to select code language
ClosedPublic
Actions

Authored by kalpitk on Jul 12 2019, 8:26 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDWAPPS292c09cc1b2d: Add language selection option
Summary

Related T1909

Diff Detail

Event Timeline

kalpitk created this revision.Jul 12 2019, 8:26 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJul 12 2019, 8:26 PM
swh-public-ci added a comment.Jul 12 2019, 8:29 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/611/ for more details.

swh-public-ci added a comment.Jul 12 2019, 8:34 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/130/ for more details.

Harbormaster completed remote builds in B6855: Diff 5815.Jul 12 2019, 8:34 PM
kalpitk updated this revision to Diff 5840.Jul 15 2019, 7:41 PM
  • Refactor updateLanguage
  • Add chosen plugin for searchable dropdown
swh-public-ci added a comment.Jul 15 2019, 7:45 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/612/ for more details.

swh-public-ci added a comment.Jul 15 2019, 7:49 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/206/ for more details.

Harbormaster completed remote builds in B6879: Diff 5840.Jul 15 2019, 7:49 PM
kalpitk updated this revision to Diff 5841.Jul 15 2019, 9:29 PM
  • add dropdown
  • import css for chosen
swh-public-ci added a comment.Jul 15 2019, 9:32 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/613/ for more details.

kalpitk added a comment.Jul 15 2019, 9:33 PM

I have added chosen-js plugin for searching and filtering of languages dropdown.

Importing chosen-js in swh/web/assets/src/bundles/vendors/index.js doesn't import the css. On adding imports for css and png separately, webpack fails. I am not able to figure out why ?

swh-public-ci added a comment.Jul 15 2019, 9:34 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/207/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/207/console

Harbormaster failed remote builds in B6880: Diff 5841!Jul 15 2019, 9:34 PM
swh-public-ci added a comment.Jul 16 2019, 10:23 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/208/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/208/console

kalpitk updated this revision to Diff 5850.Jul 16 2019, 1:05 PM
  • Fix webpack error and trigger updateLanguage with dropdown
swh-public-ci added a comment.Jul 16 2019, 1:08 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/614/ for more details.

swh-public-ci added a comment.Jul 16 2019, 1:12 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/209/ for more details.

Harbormaster completed remote builds in B6886: Diff 5850.Jul 16 2019, 1:12 PM
kalpitk added a comment.Jul 16 2019, 1:41 PM

CSS needs to be updated for the dropdown.
Rest everything is done.

swh/web/assets/config/webpack.config.development.js
297

Not sure why (\?v=\d+\.\d+\.\d+) is used here. Should this be used with images also?

311

Will directory 'img/imports' be fine for outputPath?

vlorentz added a subscriber: vlorentz.Jul 16 2019, 2:36 PM
vlorentz added inline comments.
swh/web/assets/config/webpack.config.development.js
297

I don't know... don't use it if it's not needed.

306

Do we need GIF and JPEG for now?

311

What about img/thirdParty (to match the name used elsewhere)?

swh/web/templates/includes/top-navigation.html
134

What does this line do?

kalpitk added inline comments.Jul 16 2019, 2:45 PM
swh/web/assets/config/webpack.config.development.js
297

ok

306

No. Should I remove gif and jpeg from here?

311

ok

swh/web/templates/includes/top-navigation.html
134

It was for initialising chosen on select.

It works without this line, because of the next line.
Will remove it.

vlorentz added inline comments.Jul 16 2019, 3:41 PM
swh/web/assets/config/webpack.config.development.js
306

If we don't need it, yes.

kalpitk updated this revision to Diff 5854.Jul 16 2019, 7:39 PM
  • Add css
swh-public-ci added a comment.Jul 16 2019, 7:41 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/615/ for more details.

kalpitk added a comment.Jul 16 2019, 7:43 PM

I had to use !important everywhere to override the default css. I guess, there must be a way to avoid it.

swh-public-ci added a comment.Jul 16 2019, 7:44 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/210/ for more details.

Harbormaster completed remote builds in B6890: Diff 5854.Jul 16 2019, 7:44 PM
kalpitk retitled this revision from [WIP] Add option to select code language to Add option to select code language.Jul 17 2019, 1:22 PM
kalpitk updated this revision to Diff 5857.Jul 17 2019, 1:24 PM
kalpitk retitled this revision from Add option to select code language to [WIP] Add option to select code language.

Update CSS and refactor

swh-public-ci added a comment.Jul 17 2019, 1:27 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/616/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/616/console

Harbormaster failed remote builds in B6893: Diff 5857!Jul 17 2019, 1:27 PM
swh-public-ci added a comment.Jul 17 2019, 1:29 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/211/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/211/console

kalpitk updated this revision to Diff 5859.Jul 17 2019, 2:57 PM
  • Fix failing tests
swh-public-ci added a comment.Jul 17 2019, 3:00 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/617/ for more details.

Harbormaster completed remote builds in B6895: Diff 5859.Jul 17 2019, 3:02 PM
swh-public-ci added a comment.Jul 17 2019, 3:02 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/212/ for more details.

kalpitk updated this revision to Diff 5860.Jul 17 2019, 3:04 PM

Update commit msg

kalpitk retitled this revision from [WIP] Add option to select code language to Add option to select code language.Jul 17 2019, 3:04 PM
swh-public-ci added a comment.Jul 17 2019, 3:07 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/618/ for more details.

swh-public-ci added a comment.Jul 17 2019, 3:10 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/213/ for more details.

Harbormaster completed remote builds in B6896: Diff 5860.Jul 17 2019, 3:10 PM
vlorentz added a comment.Jul 17 2019, 4:02 PM

It's really nice!

Just a few remarks:

  • it's unclear what language_select does, especially as it's used both as a list of languages and a boolean to tell whether the language selector should be shown. It should be renamed to available_languages
swh/web/browse/views/utils/snapshot_context.py
457 ↗(On Diff #5860)

"Override language with user-selected language"

swh/web/templates/includes/content-display.html
68

This currently works because there are no other params, but it would be nice to support other parameters in the URL, so we don't get unexpected bugs later.

swh/web/templates/includes/top-navigation.html
133

{{ language }} seems to be sanitized (when I try values with quotes, eg. http://127.0.0.1:5004/browse/origin/https://github.com/wcoder/highlightjs-line-numbers.js/content/gulpfile.js/?language=foo%22);%20alert(%22hello ), but where?

kalpitk added inline comments.Jul 17 2019, 8:19 PM
swh/web/browse/views/utils/snapshot_context.py
457 ↗(On Diff #5860)

ok

swh/web/templates/includes/content-display.html
68

I tested it with other params like origin_url, it works.
I mean for links like "http://127.0.0.1:5004/browse/content/sha1_git:33217f73631dbfa1860b41060e5a6a5a62420f69/?origin_url=https%3A%2F%2Fgithub.com%2Fusartkom%2Flearning_pyhon&language=gherkin", everything works.

(URLSearchParams stores all params in url)

swh/web/templates/includes/top-navigation.html
133

Django auto-escapes template variables by default.

kalpitk updated this revision to Diff 5870.Jul 17 2019, 8:28 PM

language_select renamed available_languages

swh-public-ci added a comment.Jul 17 2019, 8:31 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/619/ for more details.

swh-public-ci added a comment.Jul 17 2019, 8:33 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/214/ for more details.

Harbormaster completed remote builds in B6905: Diff 5870.Jul 17 2019, 8:33 PM
vlorentz accepted this revision.Jul 18 2019, 10:25 AM
vlorentz added inline comments.
swh/web/templates/includes/content-display.html
68

Indeed, my bad

This revision is now accepted and ready to land.Jul 18 2019, 10:25 AM
Closed by commit rDWAPPS292c09cc1b2d: Add language selection option (authored by kalpitk). · Explain WhyJul 18 2019, 6:11 PM
This revision was automatically updated to reflect the committed changes.
kalpitk mentioned this in D1756: cypress: Test Language-select.Jul 19 2019, 9:47 PM

Revision Contents
Changeset List

PathSize
1 line
swh/
web/
assets/
config/
9 lines
src/
bundles/
browse/
2 lines
vendors/
6 lines
browse/
views/
11 lines
templates/
includes/
18 lines
14 lines
5 lines

Diff 5850

View Options

package.json

Loading...
View Options

swh/web/assets/config/webpack.config.development.js

Loading...
View Options

swh/web/assets/src/bundles/browse/origin-search.js

Loading...
View Options

swh/web/assets/src/bundles/vendors/index.js

Loading...
View Options

swh/web/browse/views/content.py

Loading...
View Options

swh/web/templates/includes/content-display.html

Loading...
View Options

swh/web/templates/includes/top-navigation.html

Loading...
View Options

yarn.lock

Loading...
About · Developer information · Legal